From e9dbd2b7819efdb2a3d2f92334c2e25849068e00 Mon Sep 17 00:00:00 2001 From: Neale Date: Fri, 1 Jul 2016 13:00:09 +0100 Subject: VPP-142 Recursive route sending traffic to wrong interface Indirect routes should always result in an indriect adjacency even when the route's next-hop is covered by a connceted, since the covering route may change and no tracking is in place. Some de-duplication of code for installing indirect routes via the CLI and API. Change-Id: I7a440ffba43ae3990b68cb407244c06bd0827534 Signed-off-by: Neale --- vnet/vnet/ip/ip4.h | 7 ++ vnet/vnet/ip/ip4_forward.c | 138 +++++++++++++++++++++++---------------- vnet/vnet/ip/ip6.h | 8 +++ vnet/vnet/ip/ip6_forward.c | 147 +++++++++++++++++++++++++----------------- vpp-api-test/vat/api_format.c | 2 +- vpp/vpp-api/api.c | 132 +++++++++++++++++-------------------- 6 files changed, 249 insertions(+), 185 deletions(-) diff --git a/vnet/vnet/ip/ip4.h b/vnet/vnet/ip/ip4.h index 7ed5bf5e9a9..5ea4ebf56ef 100644 --- a/vnet/vnet/ip/ip4.h +++ b/vnet/vnet/ip/ip4.h @@ -355,6 +355,13 @@ void ip4_add_del_route_next_hop (ip4_main_t * im, u32 next_hop_weight, u32 adj_index, u32 explicit_fib_index); +u32 +ip4_route_get_next_hop_adj (ip4_main_t * im, + u32 fib_index, + ip4_address_t *next_hop, + u32 next_hop_sw_if_index, + u32 explicit_fib_index); + void * ip4_get_route (ip4_main_t * im, u32 fib_index_or_table_id, diff --git a/vnet/vnet/ip/ip4_forward.c b/vnet/vnet/ip/ip4_forward.c index 35c42d822b7..97e472b63c7 100644 --- a/vnet/vnet/ip/ip4_forward.c +++ b/vnet/vnet/ip/ip4_forward.c @@ -274,6 +274,85 @@ void ip4_add_del_route (ip4_main_t * im, ip4_add_del_route_args_t * a) ip_del_adjacency (lm, old_adj_index); } + +u32 +ip4_route_get_next_hop_adj (ip4_main_t * im, + u32 fib_index, + ip4_address_t *next_hop, + u32 next_hop_sw_if_index, + u32 explicit_fib_index) +{ + ip_lookup_main_t * lm = &im->lookup_main; + vnet_main_t * vnm = vnet_get_main(); + uword * nh_hash, * nh_result; + int is_interface_next_hop; + u32 nh_adj_index; + ip4_fib_t * fib; + + fib = vec_elt_at_index (im->fibs, fib_index); + + is_interface_next_hop = next_hop->data_u32 == 0; + if (is_interface_next_hop) + { + nh_result = hash_get (im->interface_route_adj_index_by_sw_if_index, next_hop_sw_if_index); + if (nh_result) + nh_adj_index = *nh_result; + else + { + ip_adjacency_t * adj; + adj = ip_add_adjacency (lm, /* template */ 0, /* block size */ 1, + &nh_adj_index); + ip4_adjacency_set_interface_route (vnm, adj, next_hop_sw_if_index, /* if_address_index */ ~0); + ip_call_add_del_adjacency_callbacks (lm, nh_adj_index, /* is_del */ 0); + hash_set (im->interface_route_adj_index_by_sw_if_index, next_hop_sw_if_index, nh_adj_index); + } + } + else if (next_hop_sw_if_index == ~0) + { + /* next-hop is recursive. we always need a indirect adj + * for recursive paths. Any LPM we perform now will give + * us a valid adj, but without tracking the next-hop we + * have no way to keep it valid. + */ + ip_adjacency_t add_adj; + memset (&add_adj, 0, sizeof(add_adj)); + add_adj.n_adj = 1; + add_adj.lookup_next_index = IP_LOOKUP_NEXT_INDIRECT; + add_adj.indirect.next_hop.ip4.as_u32 = next_hop->as_u32; + add_adj.explicit_fib_index = explicit_fib_index; + ip_add_adjacency (lm, &add_adj, 1, &nh_adj_index); + } + else + { + nh_hash = fib->adj_index_by_dst_address[32]; + nh_result = hash_get (nh_hash, next_hop->data_u32); + + /* Next hop must be known. */ + if (! nh_result) + { + ip_adjacency_t * adj; + + /* no /32 exists, get the longest prefix match */ + nh_adj_index = ip4_fib_lookup_with_table (im, fib_index, + next_hop, 0); + adj = ip_get_adjacency (lm, nh_adj_index); + /* if ARP interface adjacency is present, we need to + install ARP adjaceny for specific next hop */ + if (adj->lookup_next_index == IP_LOOKUP_NEXT_ARP && + adj->arp.next_hop.ip4.as_u32 == 0) + { + nh_adj_index = vnet_arp_glean_add(fib_index, next_hop); + } + } + else + { + nh_adj_index = *nh_result; + } + } + + return (nh_adj_index); +} + void ip4_add_del_route_next_hop (ip4_main_t * im, u32 flags, @@ -291,11 +370,9 @@ ip4_add_del_route_next_hop (ip4_main_t * im, u32 dst_address_u32, old_mp_adj_index, new_mp_adj_index; u32 dst_adj_index, nh_adj_index; uword * dst_hash, * dst_result; - uword * nh_hash, * nh_result; ip_adjacency_t * dst_adj; ip_multipath_adjacency_t * old_mp, * new_mp; int is_del = (flags & IP4_ROUTE_FLAG_DEL) != 0; - int is_interface_next_hop; clib_error_t * error = 0; if (explicit_fib_index == (u32)~0) @@ -304,61 +381,14 @@ ip4_add_del_route_next_hop (ip4_main_t * im, fib_index = explicit_fib_index; fib = vec_elt_at_index (im->fibs, fib_index); - + /* Lookup next hop to be added or deleted. */ - is_interface_next_hop = next_hop->data_u32 == 0; if (adj_index == (u32)~0) { - if (is_interface_next_hop) - { - nh_result = hash_get (im->interface_route_adj_index_by_sw_if_index, next_hop_sw_if_index); - if (nh_result) - nh_adj_index = *nh_result; - else - { - ip_adjacency_t * adj; - adj = ip_add_adjacency (lm, /* template */ 0, /* block size */ 1, - &nh_adj_index); - ip4_adjacency_set_interface_route (vnm, adj, next_hop_sw_if_index, /* if_address_index */ ~0); - ip_call_add_del_adjacency_callbacks (lm, nh_adj_index, /* is_del */ 0); - hash_set (im->interface_route_adj_index_by_sw_if_index, next_hop_sw_if_index, nh_adj_index); - } - } - else - { - nh_hash = fib->adj_index_by_dst_address[32]; - nh_result = hash_get (nh_hash, next_hop->data_u32); - - /* Next hop must be known. */ - if (! nh_result) - { - ip_adjacency_t * adj; - - nh_adj_index = ip4_fib_lookup_with_table (im, fib_index, - next_hop, 0); - adj = ip_get_adjacency (lm, nh_adj_index); - /* if ARP interface adjacencty is present, we need to - install ARP adjaceny for specific next hop */ - if (adj->lookup_next_index == IP_LOOKUP_NEXT_ARP && - adj->arp.next_hop.ip4.as_u32 == 0) - { - nh_adj_index = vnet_arp_glean_add(fib_index, next_hop); - } - else - { - /* Next hop is not known, so create indirect adj */ - ip_adjacency_t add_adj; - memset (&add_adj, 0, sizeof(add_adj)); - add_adj.n_adj = 1; - add_adj.lookup_next_index = IP_LOOKUP_NEXT_INDIRECT; - add_adj.indirect.next_hop.ip4.as_u32 = next_hop->as_u32; - add_adj.explicit_fib_index = explicit_fib_index; - ip_add_adjacency (lm, &add_adj, 1, &nh_adj_index); - } - } - else - nh_adj_index = *nh_result; - } + nh_adj_index = ip4_route_get_next_hop_adj(im, fib_index, + next_hop, + next_hop_sw_if_index, + explicit_fib_index); } else { diff --git a/vnet/vnet/ip/ip6.h b/vnet/vnet/ip/ip6.h index edeecdee8a6..22d1a4feaa0 100644 --- a/vnet/vnet/ip/ip6.h +++ b/vnet/vnet/ip/ip6.h @@ -389,6 +389,14 @@ void ip6_add_del_route_next_hop (ip6_main_t * im, u32 next_hop_sw_if_index, u32 next_hop_weight, u32 adj_index, u32 explicit_fib_index); + +u32 +ip6_route_get_next_hop_adj (ip6_main_t * im, + u32 fib_index, + ip6_address_t *next_hop, + u32 next_hop_sw_if_index, + u32 explicit_fib_index); + u32 ip6_get_route (ip6_main_t * im, u32 fib_index_or_table_id, diff --git a/vnet/vnet/ip/ip6_forward.c b/vnet/vnet/ip/ip6_forward.c index ced5f562be8..c581a9bc42e 100644 --- a/vnet/vnet/ip/ip6_forward.c +++ b/vnet/vnet/ip/ip6_forward.c @@ -297,6 +297,91 @@ void ip6_add_del_route (ip6_main_t * im, ip6_add_del_route_args_t * a) } } +u32 +ip6_route_get_next_hop_adj (ip6_main_t * im, + u32 fib_index, + ip6_address_t *next_hop, + u32 next_hop_sw_if_index, + u32 explicit_fib_index) +{ + ip_lookup_main_t * lm = &im->lookup_main; + vnet_main_t * vnm = vnet_get_main(); + int is_interface_next_hop; + uword * nh_result; + u32 nh_adj_index; + ip6_fib_t * fib; + + fib = vec_elt_at_index (im->fibs, fib_index); + + is_interface_next_hop = ip6_address_is_zero (next_hop); + + if (is_interface_next_hop) + { + nh_result = hash_get (im->interface_route_adj_index_by_sw_if_index, + next_hop_sw_if_index); + if (nh_result) + nh_adj_index = *nh_result; + else + { + ip_adjacency_t * adj; + adj = ip_add_adjacency (lm, /* template */ 0, /* block size */ 1, + &nh_adj_index); + ip6_adjacency_set_interface_route (vnm, adj, + next_hop_sw_if_index, ~0); + ip_call_add_del_adjacency_callbacks + (lm, next_hop_sw_if_index, /* is_del */ 0); + hash_set (im->interface_route_adj_index_by_sw_if_index, + next_hop_sw_if_index, nh_adj_index); + } + } + else if (next_hop_sw_if_index == ~0) + { + /* next-hop is recursive. we always need a indirect adj + * for recursive paths. Any LPM we perform now will give + * us a valid adj, but without tracking the next-hop we + * have no way to keep it valid. + */ + ip_adjacency_t add_adj; + memset (&add_adj, 0, sizeof(add_adj)); + add_adj.n_adj = 1; + add_adj.lookup_next_index = IP_LOOKUP_NEXT_INDIRECT; + add_adj.indirect.next_hop.ip6.as_u64[0] = next_hop->as_u64[0]; + add_adj.indirect.next_hop.ip6.as_u64[1] = next_hop->as_u64[1]; + add_adj.explicit_fib_index = explicit_fib_index; + ip_add_adjacency (lm, &add_adj, 1, &nh_adj_index); + } + else + { + BVT(clib_bihash_kv) kv, value; + + /* Look for the interface /128 route */ + kv.key[0] = next_hop->as_u64[0]; + kv.key[1] = next_hop->as_u64[1]; + kv.key[2] = ((u64)((fib - im->fibs))<<32) | 128; + + if (BV(clib_bihash_search)(&im->ip6_lookup_table, &kv, &value) < 0) + { + ip_adjacency_t * adj; + nh_adj_index = ip6_fib_lookup_with_table (im, fib_index, next_hop); + adj = ip_get_adjacency (lm, nh_adj_index); + /* if ND interface adjacencty is present, we need to + install ND adjaceny for specific next hop */ + if (adj->lookup_next_index == IP_LOOKUP_NEXT_ARP && + adj->arp.next_hop.ip6.as_u64[0] == 0 && + adj->arp.next_hop.ip6.as_u64[1] == 0) + { + nh_adj_index = vnet_ip6_neighbor_glean_add(fib_index, next_hop); + } + } + else + { + nh_adj_index = value.value; + } + } + + return (nh_adj_index); +} + void ip6_add_del_route_next_hop (ip6_main_t * im, u32 flags, @@ -318,9 +403,7 @@ ip6_add_del_route_next_hop (ip6_main_t * im, ip_adjacency_t * dst_adj; ip_multipath_adjacency_t * old_mp, * new_mp; int is_del = (flags & IP6_ROUTE_FLAG_DEL) != 0; - int is_interface_next_hop; clib_error_t * error = 0; - uword * nh_result; BVT(clib_bihash_kv) kv, value; vlib_smp_unsafe_warning(); @@ -333,64 +416,12 @@ ip6_add_del_route_next_hop (ip6_main_t * im, fib = vec_elt_at_index (im->fibs, fib_index); /* Lookup next hop to be added or deleted. */ - is_interface_next_hop = ip6_address_is_zero (next_hop); if (adj_index == (u32)~0) { - if (is_interface_next_hop) - { - nh_result = hash_get (im->interface_route_adj_index_by_sw_if_index, - next_hop_sw_if_index); - if (nh_result) - nh_adj_index = *nh_result; - else - { - ip_adjacency_t * adj; - adj = ip_add_adjacency (lm, /* template */ 0, /* block size */ 1, - &nh_adj_index); - ip6_adjacency_set_interface_route (vnm, adj, - next_hop_sw_if_index, ~0); - ip_call_add_del_adjacency_callbacks - (lm, nh_adj_index, /* is_del */ 0); - hash_set (im->interface_route_adj_index_by_sw_if_index, - next_hop_sw_if_index, nh_adj_index); - } - } - else - { - /* Look for the interface /128 route */ - kv.key[0] = next_hop->as_u64[0]; - kv.key[1] = next_hop->as_u64[1]; - kv.key[2] = ((u64)((fib - im->fibs))<<32) | 128; - - if (BV(clib_bihash_search)(&im->ip6_lookup_table, &kv, &value) < 0) - { - ip_adjacency_t * adj; - nh_adj_index = ip6_fib_lookup_with_table (im, fib_index, next_hop); - adj = ip_get_adjacency (lm, nh_adj_index); - /* if ND interface adjacencty is present, we need to - install ND adjaceny for specific next hop */ - if (adj->lookup_next_index == IP_LOOKUP_NEXT_ARP && - adj->arp.next_hop.ip6.as_u64[0] == 0 && - adj->arp.next_hop.ip6.as_u64[1] == 0) - { - nh_adj_index = vnet_ip6_neighbor_glean_add(fib_index, next_hop); - } - else - { - ip_adjacency_t add_adj; - memset (&add_adj, 0, sizeof(add_adj)); - add_adj.n_adj = 1; - add_adj.lookup_next_index = IP_LOOKUP_NEXT_INDIRECT; - add_adj.indirect.next_hop.ip6.as_u64[0] = next_hop->as_u64[0]; - add_adj.indirect.next_hop.ip6.as_u64[1] = next_hop->as_u64[1]; - add_adj.explicit_fib_index = explicit_fib_index; - ip_add_adjacency (lm, &add_adj, 1, &nh_adj_index); - } - } - else - nh_adj_index = value.value; - - } + nh_adj_index = ip6_route_get_next_hop_adj(im, fib_index, + next_hop, + next_hop_sw_if_index, + explicit_fib_index); } else { diff --git a/vpp-api-test/vat/api_format.c b/vpp-api-test/vat/api_format.c index 6b5271dd039..d4bc27bda8a 100644 --- a/vpp-api-test/vat/api_format.c +++ b/vpp-api-test/vat/api_format.c @@ -4269,7 +4269,7 @@ static int api_ip_add_del_route (vat_main_t * vam) unformat_input_t * i = vam->input; vl_api_ip_add_del_route_t *mp; f64 timeout; - u32 sw_if_index = 0, vrf_id = 0; + u32 sw_if_index = ~0, vrf_id = 0; u8 sw_if_index_set = 0; u8 is_ipv6 = 0; u8 is_local = 0, is_drop = 0; diff --git a/vpp/vpp-api/api.c b/vpp/vpp-api/api.c index 6041b11e8de..0879d49dcee 100644 --- a/vpp/vpp-api/api.c +++ b/vpp/vpp-api/api.c @@ -862,7 +862,7 @@ static int ip4_add_del_route_t_handler (vl_api_ip_add_del_route_t *mp) uword * p; clib_error_t * e; u32 ai; - ip_adjacency_t *nh_adj, *add_adj = 0; + ip_adjacency_t *adj; p = hash_get (im->fib_index_by_table_id, ntohl(mp->vrf_id)); if (!p) { @@ -879,7 +879,8 @@ static int ip4_add_del_route_t_handler (vl_api_ip_add_del_route_t *mp) fib_index = p[0]; } - if (pool_is_free_index (vnm->interface_main.sw_interfaces, + if (~0 != mp->next_hop_sw_if_index && + pool_is_free_index (vnm->interface_main.sw_interfaces, ntohl(mp->next_hop_sw_if_index))) return VNET_API_ERROR_NO_MATCHING_INTERFACE; @@ -887,7 +888,7 @@ static int ip4_add_del_route_t_handler (vl_api_ip_add_del_route_t *mp) sizeof (next_hop_address.data)); /* Arp for the next_hop if necessary */ - if (mp->is_add && mp->resolve_if_needed) { + if (mp->is_add && mp->resolve_if_needed && ~0 != mp->next_hop_sw_if_index) { u32 lookup_result; ip_adjacency_t * adj; @@ -971,50 +972,43 @@ static int ip4_add_del_route_t_handler (vl_api_ip_add_del_route_t *mp) else if (mp->is_local) ai = lm->local_adj_index; else if (mp->is_classify) { - ip_adjacency_t cadj; - memset(&cadj, 0, sizeof(cadj)); - cadj.lookup_next_index = IP_LOOKUP_NEXT_CLASSIFY; - cadj.classify.table_index = ntohl(mp->classify_table_index); - if (pool_is_free_index (cm->tables, cadj.classify.table_index)) { + if (pool_is_free_index (cm->tables, ntohl(mp->classify_table_index))) { dsunlock(sm); return VNET_API_ERROR_NO_SUCH_TABLE; } - vec_add1 (add_adj, cadj); - goto do_add_del; - } - else { - ai = ip4_fib_lookup_with_table - (im, fib_index, &next_hop_address, - 1 /* disable default route */); - if (ai == lm->miss_adj_index) { - dsunlock(sm); - return VNET_API_ERROR_NEXT_HOP_NOT_IN_FIB; - } - } + adj = ip_add_adjacency (lm, + /* template */ 0, + /* block size */ 1, + &ai); - nh_adj = ip_get_adjacency (lm, ai); - if (nh_adj->lookup_next_index == IP_LOOKUP_NEXT_ARP && - nh_adj->arp.next_hop.ip4.as_u32 == 0) { - /* the next-hop resovles via a glean adj. create and use - * a ARP adj for the next-hop */ - a.adj_index = vnet_arp_glean_add(fib_index, &next_hop_address); - a.add_adj = NULL; - a.n_add_adj = 0; - ip4_add_del_route (im, &a); - - goto done; + adj->lookup_next_index = IP_LOOKUP_NEXT_CLASSIFY; + adj->classify.table_index = ntohl(mp->classify_table_index); } - vec_add1 (add_adj, nh_adj[0]); - if (mp->lookup_in_vrf) { + else if (mp->lookup_in_vrf) { p = hash_get (im->fib_index_by_table_id, ntohl(mp->lookup_in_vrf)); - if (p) - add_adj[0].explicit_fib_index = p[0]; + if (p) { + adj = ip_add_adjacency (lm, + /* template */ 0, + /* block size */ 1, + &ai); + adj->explicit_fib_index = p[0]; + } else { - vec_free (add_adj); dsunlock(sm); - return VNET_API_ERROR_NO_SUCH_INNER_FIB; - } - } + return VNET_API_ERROR_NO_SUCH_INNER_FIB; + } + } + else + ai = ip4_route_get_next_hop_adj (im, + fib_index, + &next_hop_address, + ntohl(mp->next_hop_sw_if_index), + fib_index); + + if (ai == lm->miss_adj_index) { + dsunlock(sm); + return VNET_API_ERROR_NO_SUCH_INNER_FIB; + } } else { ip_adjacency_t * adj; int disable_default_route = 1; @@ -1038,15 +1032,9 @@ static int ip4_add_del_route_t_handler (vl_api_ip_add_del_route_t *mp) } } -do_add_del: - a.adj_index = ~0; - a.add_adj = add_adj; - a.n_add_adj = vec_len(add_adj); + a.adj_index = ai; ip4_add_del_route (im, &a); - vec_free (add_adj); - -done: dsunlock (sm); return 0; } @@ -1067,7 +1055,7 @@ static int ip6_add_del_route_t_handler (vl_api_ip_add_del_route_t *mp) u32 fib_index; uword * p; clib_error_t * e; - ip_adjacency_t * nh_adj, * add_adj = 0; + ip_adjacency_t *adj = 0; u32 ai; p = hash_get (im->fib_index_by_table_id, ntohl(mp->vrf_id)); @@ -1086,7 +1074,8 @@ static int ip6_add_del_route_t_handler (vl_api_ip_add_del_route_t *mp) fib_index = p[0]; } - if (pool_is_free_index (vnm->interface_main.sw_interfaces, + if (~0 != mp->next_hop_sw_if_index && + pool_is_free_index (vnm->interface_main.sw_interfaces, ntohl(mp->next_hop_sw_if_index))) return VNET_API_ERROR_NO_MATCHING_INTERFACE; @@ -1094,7 +1083,7 @@ static int ip6_add_del_route_t_handler (vl_api_ip_add_del_route_t *mp) sizeof (next_hop_address.as_u8)); /* Arp for the next_hop if necessary */ - if (mp->is_add && mp->resolve_if_needed) { + if (mp->is_add && mp->resolve_if_needed && ~0 != mp->next_hop_sw_if_index) { u32 lookup_result; ip_adjacency_t * adj; @@ -1176,27 +1165,30 @@ static int ip6_add_del_route_t_handler (vl_api_ip_add_del_route_t *mp) ai = lm->drop_adj_index; else if (mp->is_local) ai = lm->local_adj_index; - else { - ai = ip6_fib_lookup_with_table - (im, fib_index, &next_hop_address); - if (ai == lm->miss_adj_index) { - dsunlock(sm); - return VNET_API_ERROR_NEXT_HOP_NOT_IN_FIB; - } - } - - nh_adj = ip_get_adjacency (lm, ai); - vec_add1 (add_adj, nh_adj[0]); - if (mp->lookup_in_vrf) { + else if (mp->lookup_in_vrf) { p = hash_get (im->fib_index_by_table_id, ntohl(mp->lookup_in_vrf)); - if (p) - add_adj[0].explicit_fib_index = p[0]; + if (p) { + adj = ip_add_adjacency (lm, + /* template */ 0, + /* block size */ 1, + &ai); + adj->explicit_fib_index = p[0]; + } else { - vec_free (add_adj); - dsunlock(sm); + dsunlock(sm); return VNET_API_ERROR_NO_SUCH_INNER_FIB; - } - } + } + } + else + ai = ip6_route_get_next_hop_adj (im, + fib_index, + &next_hop_address, + ntohl(mp->next_hop_sw_if_index), + fib_index); + if (ai == lm->miss_adj_index) { + dsunlock(sm); + return VNET_API_ERROR_NEXT_HOP_NOT_IN_FIB; + } } else { ip_adjacency_t * adj; @@ -1213,13 +1205,9 @@ static int ip6_add_del_route_t_handler (vl_api_ip_add_del_route_t *mp) } } - a.adj_index = ~0; - a.add_adj = add_adj; - a.n_add_adj = vec_len(add_adj); + a.adj_index = ai; ip6_add_del_route (im, &a); - vec_free (add_adj); - dsunlock (sm); return 0; } -- cgit 1.2.3-korg