From 53962fbccb4612af41e3fbd913cc50be09d3b24a Mon Sep 17 00:00:00 2001 From: Neale Ranns Date: Mon, 20 Dec 2021 18:18:42 +0000 Subject: [PATCH] fib: MPLS EOS chains built for attached prefixes should link to a lookup DPO Type: fix Presently a local label associated with an attached or connected prefix will link to the glean. This is a problem since it will never use the adj-fibs that are installed for that attached prefix. Instead link the local label to a lookup in the table in which the attached link is bound. Signed-off-by: Neale Ranns Change-Id: Iad49fb6168b9ba47216a9a52bd262363b49c3c43 --- src/vnet/fib/fib_entry_src.c | 56 +++++++------------------------------------- src/vnet/fib/fib_entry_src.h | 3 --- src/vnet/fib/fib_path.c | 38 +++++++++++++++++++++++++----- src/vnet/fib/fib_path.h | 2 ++ src/vnet/fib/fib_path_ext.c | 5 ++-- src/vnet/fib/fib_path_ext.h | 2 +- src/vnet/fib/fib_path_list.c | 6 +++-- src/vnet/mfib/mfib_entry.c | 5 +++- src/vnet/mpls/mpls_tunnel.c | 6 ++--- test/test_mpls.py | 27 +++++++++++++++++++++ 10 files changed, 84 insertions(+), 66 deletions(-) diff --git a/src/vnet/fib/fib_entry_src.c b/src/vnet/fib/fib_entry_src.c index 5e66de7c93b..c0275e898bd 100644 --- a/src/vnet/fib/fib_entry_src.c +++ b/src/vnet/fib/fib_entry_src.c @@ -257,6 +257,7 @@ typedef struct fib_entry_src_collect_forwarding_ctx_t_ fib_forward_chain_type_t fct; int n_recursive_constrained; u16 preference; + dpo_proto_t payload_proto; } fib_entry_src_collect_forwarding_ctx_t; /** @@ -289,47 +290,6 @@ fib_entry_src_valid_out_label (mpls_label_t label) MPLS_IETF_IMPLICIT_NULL_LABEL == label)); } -/** - * @brief Turn the chain type requested by the client into the one they - * really wanted - */ -fib_forward_chain_type_t -fib_entry_chain_type_fixup (const fib_entry_t *entry, - fib_forward_chain_type_t fct) -{ - /* - * The EOS chain is a tricky since one cannot know the adjacency - * to link to without knowing what the packets payload protocol - * will be once the label is popped. - */ - fib_forward_chain_type_t dfct; - - if (FIB_FORW_CHAIN_TYPE_MPLS_EOS != fct) - { - return (fct); - } - - dfct = fib_entry_get_default_chain_type(entry); - - if (FIB_FORW_CHAIN_TYPE_MPLS_EOS == dfct) - { - /* - * If the entry being asked is a eos-MPLS label entry, - * then use the payload-protocol field, that we stashed there - * for just this purpose - */ - return (fib_forw_chain_type_from_dpo_proto( - entry->fe_prefix.fp_payload_proto)); - } - /* - * else give them what this entry would be by default. i.e. if it's a v6 - * entry, then the label its local labelled should be carrying v6 traffic. - * If it's a non-EOS label entry, then there are more labels and we want - * a non-eos chain. - */ - return (dfct); -} - static dpo_proto_t fib_prefix_get_payload_proto (const fib_prefix_t *pfx) { @@ -371,7 +331,8 @@ fib_entry_src_get_path_forwarding (fib_node_index_t path_index, nh->path_index = path_index; nh->path_weight = fib_path_get_weight(path_index); - fib_path_contribute_forwarding(path_index, ctx->fct, &nh->path_dpo); + fib_path_contribute_forwarding(path_index, ctx->fct, + ctx->payload_proto, &nh->path_dpo); break; case FIB_FORW_CHAIN_TYPE_MPLS_NON_EOS: @@ -384,6 +345,7 @@ fib_entry_src_get_path_forwarding (fib_node_index_t path_index, nh->path_weight = fib_path_get_weight(path_index); fib_path_contribute_forwarding(path_index, FIB_FORW_CHAIN_TYPE_MPLS_NON_EOS, + ctx->payload_proto, &nh->path_dpo); } break; @@ -397,11 +359,11 @@ fib_entry_src_get_path_forwarding (fib_node_index_t path_index, nh->path_index = path_index; nh->path_weight = fib_path_get_weight(path_index); fib_path_contribute_forwarding(path_index, - fib_entry_chain_type_fixup(ctx->fib_entry, - ctx->fct), + ctx->fct, + ctx->payload_proto, &nh->path_dpo); fib_path_stack_mpls_disp(path_index, - fib_prefix_get_payload_proto(&ctx->fib_entry->fe_prefix), + ctx->payload_proto, FIB_MPLS_LSP_MODE_PIPE, &nh->path_dpo); @@ -480,9 +442,8 @@ fib_entry_src_collect_forwarding (fib_node_index_t pl_index, */ ctx->next_hops = fib_path_ext_stack(path_ext, + ctx->payload_proto, ctx->fct, - fib_entry_chain_type_fixup(ctx->fib_entry, - ctx->fct), ctx->next_hops); } else @@ -609,6 +570,7 @@ fib_entry_src_mk_lb (fib_entry_t *fib_entry, .preference = 0xffff, .start_source_index = start, .end_source_index = end, + .payload_proto = fib_prefix_get_payload_proto(&fib_entry->fe_prefix), }; /* diff --git a/src/vnet/fib/fib_entry_src.h b/src/vnet/fib/fib_entry_src.h index ced6b5c42fc..1f348baeacb 100644 --- a/src/vnet/fib/fib_entry_src.h +++ b/src/vnet/fib/fib_entry_src.h @@ -326,9 +326,6 @@ extern fib_entry_flag_t fib_entry_get_flags_i(const fib_entry_t *fib_entry); extern fib_path_list_flags_t fib_entry_src_flags_2_path_list_flags( fib_entry_flag_t eflags); -extern fib_forward_chain_type_t fib_entry_chain_type_fixup(const fib_entry_t *entry, - fib_forward_chain_type_t fct); - extern void fib_entry_src_mk_lb (fib_entry_t *fib_entry, fib_source_t source, fib_forward_chain_type_t fct, diff --git a/src/vnet/fib/fib_path.c b/src/vnet/fib/fib_path.c index d7b6091325b..e4ad8770449 100644 --- a/src/vnet/fib/fib_path.c +++ b/src/vnet/fib/fib_path.c @@ -2421,6 +2421,7 @@ fib_path_stack_mpls_disp (fib_node_index_t path_index, void fib_path_contribute_forwarding (fib_node_index_t path_index, fib_forward_chain_type_t fct, + dpo_proto_t payload_proto, dpo_id_t *dpo) { fib_path_t *path; @@ -2428,7 +2429,6 @@ fib_path_contribute_forwarding (fib_node_index_t path_index, path = fib_path_get(path_index); ASSERT(path); - ASSERT(FIB_FORW_CHAIN_TYPE_MPLS_EOS != fct); /* * The DPO stored in the path was created when the path was resolved. @@ -2446,9 +2446,19 @@ fib_path_contribute_forwarding (fib_node_index_t path_index, case FIB_PATH_TYPE_ATTACHED_NEXT_HOP: switch (fct) { + case FIB_FORW_CHAIN_TYPE_MPLS_EOS: { + dpo_id_t tmp = DPO_INVALID; + dpo_copy (&tmp, dpo); + path = fib_path_attached_next_hop_get_adj( + path, + dpo_proto_to_link(payload_proto), + &tmp); + dpo_copy (dpo, &tmp); + dpo_reset(&tmp); + break; + } case FIB_FORW_CHAIN_TYPE_UNICAST_IP4: case FIB_FORW_CHAIN_TYPE_UNICAST_IP6: - case FIB_FORW_CHAIN_TYPE_MPLS_EOS: case FIB_FORW_CHAIN_TYPE_MPLS_NON_EOS: case FIB_FORW_CHAIN_TYPE_ETHERNET: case FIB_FORW_CHAIN_TYPE_NSH: @@ -2560,10 +2570,25 @@ fib_path_contribute_forwarding (fib_node_index_t path_index, case FIB_PATH_TYPE_ATTACHED: switch (fct) { + case FIB_FORW_CHAIN_TYPE_MPLS_EOS: + /* + * End of stack traffic via an attacehd path (a glean) + * must forace an IP lookup so that the IP packet can + * match against any installed adj-fibs + */ + lookup_dpo_add_or_lock_w_fib_index( + fib_table_get_index_for_sw_if_index( + dpo_proto_to_fib(payload_proto), + path->attached.fp_interface), + payload_proto, + LOOKUP_UNICAST, + LOOKUP_INPUT_DST_ADDR, + LOOKUP_TABLE_FROM_CONFIG, + dpo); + break; case FIB_FORW_CHAIN_TYPE_MPLS_NON_EOS: case FIB_FORW_CHAIN_TYPE_UNICAST_IP4: case FIB_FORW_CHAIN_TYPE_UNICAST_IP6: - case FIB_FORW_CHAIN_TYPE_MPLS_EOS: case FIB_FORW_CHAIN_TYPE_ETHERNET: case FIB_FORW_CHAIN_TYPE_NSH: case FIB_FORW_CHAIN_TYPE_BIER: @@ -2609,8 +2634,8 @@ fib_path_contribute_forwarding (fib_node_index_t path_index, /* * Create the adj needed for sending IP multicast traffic */ - interface_rx_dpo_add_or_lock(fib_forw_chain_type_to_dpo_proto(fct), - path->attached.fp_interface, + interface_rx_dpo_add_or_lock(payload_proto, + path->intf_rx.fp_interface, dpo); break; case FIB_PATH_TYPE_UDP_ENCAP: @@ -2630,6 +2655,7 @@ fib_path_contribute_forwarding (fib_node_index_t path_index, load_balance_path_t * fib_path_append_nh_for_multipath_hash (fib_node_index_t path_index, fib_forward_chain_type_t fct, + dpo_proto_t payload_proto, load_balance_path_t *hash_key) { load_balance_path_t *mnh; @@ -2646,7 +2672,7 @@ fib_path_append_nh_for_multipath_hash (fib_node_index_t path_index, if (fib_path_is_resolved(path_index)) { - fib_path_contribute_forwarding(path_index, fct, &mnh->path_dpo); + fib_path_contribute_forwarding(path_index, fct, payload_proto, &mnh->path_dpo); } else { diff --git a/src/vnet/fib/fib_path.h b/src/vnet/fib/fib_path.h index c0f76411390..f3442c23dd6 100644 --- a/src/vnet/fib/fib_path.h +++ b/src/vnet/fib/fib_path.h @@ -191,6 +191,7 @@ extern uword fib_path_hash(fib_node_index_t path_index); extern load_balance_path_t * fib_path_append_nh_for_multipath_hash( fib_node_index_t path_index, fib_forward_chain_type_t fct, + dpo_proto_t payload_proto, load_balance_path_t *hash_key); extern void fib_path_stack_mpls_disp(fib_node_index_t path_index, dpo_proto_t payload_proto, @@ -198,6 +199,7 @@ extern void fib_path_stack_mpls_disp(fib_node_index_t path_index, dpo_id_t *dpo); extern void fib_path_contribute_forwarding(fib_node_index_t path_index, fib_forward_chain_type_t type, + dpo_proto_t payload_proto, dpo_id_t *dpo); extern void fib_path_contribute_urpf(fib_node_index_t path_index, index_t urpf); diff --git a/src/vnet/fib/fib_path_ext.c b/src/vnet/fib/fib_path_ext.c index 209b6273a85..f5611f92271 100644 --- a/src/vnet/fib/fib_path_ext.c +++ b/src/vnet/fib/fib_path_ext.c @@ -163,8 +163,8 @@ fib_path_ext_mpls_flags_to_mpls_label (fib_path_ext_mpls_flags_t fpe_flags) load_balance_path_t * fib_path_ext_stack (fib_path_ext_t *path_ext, + dpo_proto_t payload_proto, fib_forward_chain_type_t child_fct, - fib_forward_chain_type_t imp_null_fct, load_balance_path_t *nhs) { fib_forward_chain_type_t parent_fct; @@ -189,7 +189,7 @@ fib_path_ext_stack (fib_path_ext_t *path_ext, */ if (fib_path_ext_is_imp_null(path_ext)) { - parent_fct = imp_null_fct; + parent_fct = fib_forw_chain_type_from_dpo_proto(payload_proto); } else { @@ -240,6 +240,7 @@ fib_path_ext_stack (fib_path_ext_t *path_ext, */ fib_path_contribute_forwarding(path_ext->fpe_path_index, parent_fct, + payload_proto, &via_dpo); if (dpo_is_drop(&via_dpo) || diff --git a/src/vnet/fib/fib_path_ext.h b/src/vnet/fib/fib_path_ext.h index b49fd977a20..2850a588608 100644 --- a/src/vnet/fib/fib_path_ext.h +++ b/src/vnet/fib/fib_path_ext.h @@ -141,8 +141,8 @@ extern void fib_path_ext_resolve(fib_path_ext_t *path_ext, fib_node_index_t path_list_index); extern load_balance_path_t *fib_path_ext_stack(fib_path_ext_t *path_ext, + dpo_proto_t payload_proto, fib_forward_chain_type_t fct, - fib_forward_chain_type_t imp_null_fct, load_balance_path_t *nhs); extern fib_path_ext_t * fib_path_ext_list_push_back (fib_path_ext_list_t *list, diff --git a/src/vnet/fib/fib_path_list.c b/src/vnet/fib/fib_path_list.c index 81751695f47..d7e860ecb8c 100644 --- a/src/vnet/fib/fib_path_list.c +++ b/src/vnet/fib/fib_path_list.c @@ -378,8 +378,10 @@ fib_path_list_mk_lb (fib_path_list_t *path_list, if ((flags & FIB_PATH_LIST_FWD_FLAG_STICKY) || fib_path_is_resolved(*path_index)) { - nhs = fib_path_append_nh_for_multipath_hash(*path_index, - fct, nhs); + nhs = fib_path_append_nh_for_multipath_hash( + *path_index, fct, + fib_forw_chain_type_to_dpo_proto(fct), + nhs); } } diff --git a/src/vnet/mfib/mfib_entry.c b/src/vnet/mfib/mfib_entry.c index 1b685d68482..8050882d436 100644 --- a/src/vnet/mfib/mfib_entry.c +++ b/src/vnet/mfib/mfib_entry.c @@ -547,6 +547,7 @@ typedef struct mfib_entry_collect_forwarding_ctx_t_ load_balance_path_t * next_hops; fib_forward_chain_type_t fct; mfib_entry_src_t *msrc; + dpo_proto_t payload_proto; } mfib_entry_collect_forwarding_ctx_t; static fib_path_list_walk_rc_t @@ -592,7 +593,8 @@ mfib_entry_src_collect_forwarding (fib_node_index_t pl_index, nh->path_index = path_index; nh->path_weight = fib_path_get_weight(path_index); - fib_path_contribute_forwarding(path_index, ctx->fct, &nh->path_dpo); + fib_path_contribute_forwarding(path_index, ctx->fct, + ctx->payload_proto, &nh->path_dpo); break; case FIB_FORW_CHAIN_TYPE_UNICAST_IP4: @@ -632,6 +634,7 @@ mfib_entry_stack (mfib_entry_t *mfib_entry, .next_hops = NULL, .fct = mfib_entry_get_default_chain_type(mfib_entry), .msrc = msrc, + .payload_proto = fib_proto_to_dpo(mfib_entry->mfe_prefix.fp_proto), }; /* diff --git a/src/vnet/mpls/mpls_tunnel.c b/src/vnet/mpls/mpls_tunnel.c index 4715958e99c..5f7bf8c3b25 100644 --- a/src/vnet/mpls/mpls_tunnel.c +++ b/src/vnet/mpls/mpls_tunnel.c @@ -265,10 +265,8 @@ mpls_tunnel_collect_forwarding (fib_node_index_t pl_index, * found a matching extension. stack it to obtain the forwarding * info for this path. */ - ctx->next_hops = fib_path_ext_stack(path_ext, - ctx->fct, - ctx->fct, - ctx->next_hops); + ctx->next_hops = + fib_path_ext_stack (path_ext, DPO_PROTO_MPLS, ctx->fct, ctx->next_hops); return (FIB_PATH_LIST_WALK_CONTINUE); } diff --git a/test/test_mpls.py b/test/test_mpls.py index 2b959e48fb6..34645a9fce1 100644 --- a/test/test_mpls.py +++ b/test/test_mpls.py @@ -1563,6 +1563,33 @@ class TestMPLS(VppTestCase): VppMplsLabel(32), VppMplsLabel(99)]) + def test_attached(self): + """ Attach Routes with Local Label """ + + # + # test that if a local label is associated with an attached/connected + # prefix, that we can reach hosts in the prefix. + # + binding = VppMplsIpBind(self, 44, + self.pg0._local_ip4_subnet, + self.pg0.local_ip4_prefix_len) + binding.add_vpp_config() + + tx = (Ether(src=self.pg1.remote_mac, + dst=self.pg1.local_mac) / + MPLS(label=44, ttl=64) / + IP(src=self.pg0.remote_ip4, dst=self.pg0.remote_ip4) / + UDP(sport=1234, dport=1234) / + Raw(b'\xa5' * 100)) + rxs = self.send_and_expect(self.pg0, [tx], self.pg0) + for rx in rxs: + # if there's an ARP then the label is linked to the glean + # which is wrong. + self.assertFalse(rx.haslayer(ARP)) + # it should be unicasted to the host + self.assertEqual(rx[Ether].dst, self.pg0.remote_mac) + self.assertEqual(rx[IP].dst, self.pg0.remote_ip4) + class TestMPLSDisabled(VppTestCase): """ MPLS disabled """ -- 2.16.6