From 9cdd7fdaf3ce5723422114a1209c3bae2af0872b Mon Sep 17 00:00:00 2001 From: Mauro Sardara Date: Fri, 20 Jan 2023 18:19:03 +0000 Subject: fix(hicn-plugin): handle inflight interest coming from deleted face Ticket: HICN-830 Change-Id: I14ed32bba2b07575ee604850080309706eb8ce85 Signed-off-by: Mauro Sardara (cherry picked from commit 64fa723904d35eda3406058469b890a39c3f628c) --- hicn-plugin/src/data_fwd_node.c | 10 ++++- hicn-plugin/src/faces/face.h | 92 ++++++++++++++++++++++++++------------- hicn-plugin/src/faces/face_node.c | 49 +++++++++++++++------ hicn-plugin/src/pcs.h | 10 +++++ 4 files changed, 115 insertions(+), 46 deletions(-) (limited to 'hicn-plugin') diff --git a/hicn-plugin/src/data_fwd_node.c b/hicn-plugin/src/data_fwd_node.c index dc1510fae..e3466c904 100644 --- a/hicn-plugin/src/data_fwd_node.c +++ b/hicn-plugin/src/data_fwd_node.c @@ -371,7 +371,15 @@ hicn_data_node_fn (vlib_main_t *vm, vlib_node_runtime_t *node, // Get PCS entry pcs_entry = - hicn_pcs_entry_get_entry_from_index (rt->pitcs, pcs_entry_id); + hicn_pcs_entry_get_entry_from_index_safe (rt->pitcs, pcs_entry_id); + + if (PREDICT_FALSE (pcs_entry == NULL)) + { + drop_packet (vm, bi0, &n_left_to_next, &next0, &to_next, + &next_index, node); + + goto end_processing; + } isv6 = hicn_buffer_is_v6 (b0); diff --git a/hicn-plugin/src/faces/face.h b/hicn-plugin/src/faces/face.h index 99e3071af..b23cb9b12 100644 --- a/hicn-plugin/src/faces/face.h +++ b/hicn-plugin/src/faces/face.h @@ -518,6 +518,31 @@ hicn4_iface_adj_walk_cb (adj_index_t ai, void *ctx) return (ADJ_WALK_RC_CONTINUE); } +always_inline int +hicn_face_ip4_find (hicn_face_id_t *index, u8 *hicnb_flags, + const ip4_address_t *nat_addr, u32 sw_if, u32 adj_index, + u32 node_index) +{ + int ret = HICN_ERROR_FACE_NOT_FOUND; + + /*All (complete) faces are indexed by remote addess as well */ + /* if the face exists, it adds a lock */ + hicn_face_t *face = hicn_face_get ((const ip46_address_t *) nat_addr, sw_if, + &hicn_face_hashtb, adj_index); + + if (face != NULL) + { + /* unlock the face. We don't take a lock on each interest we receive */ + hicn_face_id_t face_id = hicn_dpoi_get_index (face); + hicn_face_unlock_with_id (face_id); + ret = HICN_ERROR_FACE_ALREADY_CREATED; + *hicnb_flags = HICN_BUFFER_FLAGS_DEFAULT; + *index = hicn_dpoi_get_index (face); + } + + return ret; +} + /** * @brief Retrieve, or create if it doesn't exist, a face from the ip6 local * address and returns its dpo. This method adds a lock on the face state. @@ -535,17 +560,16 @@ hicn_face_ip4_add_and_lock (hicn_face_id_t *index, u8 *hicnb_flags, u32 adj_index, u32 node_index) { int ret = HICN_ERROR_NONE; - /*All (complete) faces are indexed by remote addess as well */ - - ip46_address_t ip_address = { 0 }; - ip46_address_set_ip4 (&ip_address, nat_addr); + hicn_face_t *face = NULL; - /* if the face exists, it adds a lock */ - hicn_face_t *face = - hicn_face_get (&ip_address, sw_if, &hicn_face_hashtb, adj_index); + ret = hicn_face_ip4_find (index, hicnb_flags, nat_addr, sw_if, adj_index, + node_index); - if (face == NULL) + if (ret == HICN_ERROR_FACE_NOT_FOUND) { + ip46_address_t ip_address = { 0 }; + ip46_address_set_ip4 (&ip_address, nat_addr); + hicn_face_id_t idx; u8 face_flags = 0; @@ -566,15 +590,6 @@ hicn_face_ip4_add_and_lock (hicn_face_id_t *index, u8 *hicnb_flags, *index = idx; } - else - { - /* unlock the face. We don't take a lock on each interest we receive */ - hicn_face_id_t face_id = hicn_dpoi_get_index (face); - hicn_face_unlock_with_id (face_id); - ret = HICN_ERROR_FACE_ALREADY_CREATED; - *index = hicn_dpoi_get_index (face); - *hicnb_flags = HICN_BUFFER_FLAGS_DEFAULT; - } return ret; } @@ -600,6 +615,29 @@ hicn6_iface_adj_walk_cb (adj_index_t ai, void *ctx) return (ADJ_WALK_RC_CONTINUE); } +always_inline int +hicn_face_ip6_find (hicn_face_id_t *index, u8 *hicnb_flags, + const ip6_address_t *nat_addr, u32 sw_if, u32 adj_index, + u32 node_index) +{ + int ret = HICN_ERROR_FACE_NOT_FOUND; + + hicn_face_t *face = hicn_face_get ((const ip46_address_t *) nat_addr, sw_if, + &hicn_face_hashtb, adj_index); + + if (face != NULL) + { + /* unlock the face. We don't take a lock on each interest we receive */ + hicn_face_id_t face_id = hicn_dpoi_get_index (face); + hicn_face_unlock_with_id (face_id); + ret = HICN_ERROR_FACE_ALREADY_CREATED; + *hicnb_flags = HICN_BUFFER_FLAGS_DEFAULT; + *index = hicn_dpoi_get_index (face); + } + + return ret; +} + /** * @brief Retrieve, or create if it doesn't exist, a face from the ip6 local * address and returns its dpo. This method adds a lock on the face state. @@ -617,14 +655,15 @@ hicn_face_ip6_add_and_lock (hicn_face_id_t *index, u8 *hicnb_flags, u32 adj_index, u32 node_index) { int ret = HICN_ERROR_NONE; + hicn_face_t *face = NULL; - /*All (complete) faces are indexed by remote addess as well */ - /* if the face exists, it adds a lock */ - hicn_face_t *face = hicn_face_get ((const ip46_address_t *) nat_addr, sw_if, - &hicn_face_hashtb, adj_index); + ret = hicn_face_ip6_find (index, hicnb_flags, nat_addr, sw_if, adj_index, + node_index); - if (face == NULL) + if (ret == HICN_ERROR_FACE_NOT_FOUND) { + ip46_address_t ip_address = { 0 }; + ip46_address_set_ip6 (&ip_address, nat_addr); hicn_face_id_t idx; u8 face_flags = 0; @@ -640,15 +679,6 @@ hicn_face_ip6_add_and_lock (hicn_face_id_t *index, u8 *hicnb_flags, *hicnb_flags = HICN_BUFFER_FLAGS_NEW_FACE; *index = idx; } - else - { - /* unlock the face. We don't take a lock on each interest we receive */ - hicn_face_id_t face_id = hicn_dpoi_get_index (face); - hicn_face_unlock_with_id (face_id); - ret = HICN_ERROR_FACE_ALREADY_CREATED; - *hicnb_flags = HICN_BUFFER_FLAGS_DEFAULT; - *index = hicn_dpoi_get_index (face); - } return ret; } diff --git a/hicn-plugin/src/faces/face_node.c b/hicn-plugin/src/faces/face_node.c index 0d2e70fbe..ab51293b3 100644 --- a/hicn-plugin/src/faces/face_node.c +++ b/hicn-plugin/src/faces/face_node.c @@ -164,12 +164,15 @@ typedef enum (from_tunnel0) * ~0 + \ (1 - from_tunnel0) * vnet_buffer (b0)->sw_if_index[VLIB_RX]; \ \ - ret0 = hicn_face_ip##ipv##_add_and_lock ( \ + ret0 = hicn_face_ip##ipv##_find ( \ &hicnb0->face_id, &hicnb0->flags, &ip_hdr->dst_address, sw_if0, \ vnet_buffer (b0)->ip.adj_index[VLIB_RX], \ /* Should not be used */ ~0); \ /* Make sure the face is not created here */ \ - ASSERT (ret0 == HICN_ERROR_FACE_ALREADY_CREATED); \ + if (PREDICT_FALSE (ret0 == HICN_ERROR_FACE_NOT_FOUND)) \ + { \ + next0 = HICN##ipv##_FACE_INPUT_NEXT_ERROR_DROP; \ + } \ } \ \ vlib_increment_combined_counter ( \ @@ -261,12 +264,15 @@ typedef enum (from_tunnel0) * ~0 + \ (1 - from_tunnel0) * vnet_buffer (b0)->sw_if_index[VLIB_RX]; \ \ - ret0 = hicn_face_ip##ipv##_add_and_lock ( \ + ret0 = hicn_face_ip##ipv##_find ( \ &hicnb0->face_id, &hicnb0->flags, &ip_hdr0->dst_address, sw_if0, \ vnet_buffer (b0)->ip.adj_index[VLIB_RX], \ /* Should not be used */ ~0); \ /* Make sure the face is not created here */ \ - ASSERT (ret0 == HICN_ERROR_FACE_ALREADY_CREATED); \ + if (PREDICT_FALSE (ret0 == HICN_ERROR_FACE_NOT_FOUND)) \ + { \ + next0 = HICN##ipv##_FACE_INPUT_NEXT_ERROR_DROP; \ + } \ \ from_tunnel1 = \ (hicnb1->flags & HICN_BUFFER_FLAGS_FROM_UDP4_TUNNEL || \ @@ -275,12 +281,15 @@ typedef enum (from_tunnel1) * ~0 + \ (1 - from_tunnel1) * vnet_buffer (b1)->sw_if_index[VLIB_RX]; \ \ - ret1 = hicn_face_ip##ipv##_add_and_lock ( \ + ret1 = hicn_face_ip##ipv##_find ( \ &hicnb1->face_id, &hicnb1->flags, &ip_hdr1->dst_address, sw_if1, \ vnet_buffer (b1)->ip.adj_index[VLIB_RX], \ /* Should not be used */ ~0); \ /* Make sure the face is not created here */ \ - ASSERT (ret1 == HICN_ERROR_FACE_ALREADY_CREATED); \ + if (PREDICT_FALSE (ret1 == HICN_ERROR_FACE_NOT_FOUND)) \ + { \ + next1 = HICN##ipv##_FACE_INPUT_NEXT_ERROR_DROP; \ + } \ } \ else if (ret0 && !ret1) \ { \ @@ -292,14 +301,20 @@ typedef enum (from_tunnel0) * ~0 + \ (1 - from_tunnel0) * vnet_buffer (b0)->sw_if_index[VLIB_RX]; \ \ - ret0 = hicn_face_ip##ipv##_add_and_lock ( \ + ret0 = hicn_face_ip##ipv##_find ( \ &hicnb0->face_id, &hicnb0->flags, &ip_hdr0->dst_address, sw_if0, \ vnet_buffer (b0)->ip.adj_index[VLIB_RX], \ /* Should not be used */ ~0); \ /* Make sure the face is not created here */ \ - ASSERT (ret0 == HICN_ERROR_FACE_ALREADY_CREATED); \ - next0 = is_icmp0 * NEXT_MAPME_IP##ipv + \ - (1 - is_icmp0) * NEXT_DATA_IP##ipv; \ + if (PREDICT_FALSE (ret0 == HICN_ERROR_FACE_NOT_FOUND)) \ + { \ + next0 = HICN##ipv##_FACE_INPUT_NEXT_ERROR_DROP; \ + } \ + else \ + { \ + next0 = is_icmp0 * NEXT_MAPME_IP##ipv + \ + (1 - is_icmp0) * NEXT_DATA_IP##ipv; \ + } \ } \ else if (!ret0 && ret1) \ { \ @@ -311,14 +326,20 @@ typedef enum (from_tunnel1) * ~0 + \ (1 - from_tunnel1) * vnet_buffer (b1)->sw_if_index[VLIB_RX]; \ \ - ret1 = hicn_face_ip##ipv##_add_and_lock ( \ + ret1 = hicn_face_ip##ipv##_find ( \ &hicnb1->face_id, &hicnb1->flags, &ip_hdr1->dst_address, sw_if1, \ vnet_buffer (b1)->ip.adj_index[VLIB_RX], \ /* Should not be used */ ~0); \ /* Make sure the face is not created here */ \ - ASSERT (ret1 == HICN_ERROR_FACE_ALREADY_CREATED); \ - next1 = is_icmp1 * NEXT_MAPME_IP##ipv + \ - (1 - is_icmp1) * NEXT_DATA_IP##ipv; \ + if (PREDICT_FALSE (ret1 == HICN_ERROR_FACE_NOT_FOUND)) \ + { \ + next1 = HICN##ipv##_FACE_INPUT_NEXT_ERROR_DROP; \ + } \ + else \ + { \ + next1 = is_icmp1 * NEXT_MAPME_IP##ipv + \ + (1 - is_icmp1) * NEXT_DATA_IP##ipv; \ + } \ } \ else \ { \ diff --git a/hicn-plugin/src/pcs.h b/hicn-plugin/src/pcs.h index 0b7635100..86fb72cc9 100644 --- a/hicn-plugin/src/pcs.h +++ b/hicn-plugin/src/pcs.h @@ -377,6 +377,16 @@ hicn_pcs_entry_get_entry_from_index (const hicn_pit_cs_t *pitcs, u32 index) return pool_elt_at_index (pitcs->pcs_entries_pool, index); } +always_inline hicn_pcs_entry_t * +hicn_pcs_entry_get_entry_from_index_safe (const hicn_pit_cs_t *pitcs, + u32 index) +{ + if (!pool_is_free_index (pitcs->pcs_entries_pool, index)) + return pool_elt_at_index (pitcs->pcs_entries_pool, index); + + return NULL; +} + /* * Check if pcs entry is a content store entry */ -- cgit 1.2.3-korg