From 8db1de83ec540e01bb0577b726770bbb2338edcb Mon Sep 17 00:00:00 2001 From: Eyal Bari Date: Fri, 31 Mar 2017 02:15:17 +0300 Subject: [PATCH] ARP/API:protect against identical registrations Change-Id: Ia3acf87d3e07a7d41c047869de504e1972334b55 Signed-off-by: Eyal Bari --- src/vnet/api_errno.h | 3 +- src/vnet/ethernet/arp.c | 93 ++++++++++++++++++++----------------------------- src/vpp/api/api.c | 15 ++++---- 3 files changed, 49 insertions(+), 62 deletions(-) diff --git a/src/vnet/api_errno.h b/src/vnet/api_errno.h index 74d39bdbe68..f3ffd2a6b3e 100644 --- a/src/vnet/api_errno.h +++ b/src/vnet/api_errno.h @@ -104,7 +104,8 @@ _(BFD_EAGAIN, -111, "BFD object cannot be manipulated at this time") \ _(INVALID_GPE_MODE, -112, "Invalid GPE mode") \ _(LISP_GPE_ENTRIES_PRESENT, -113, "LISP GPE entries are present") \ _(ADDRESS_FOUND_FOR_INTERFACE, -114, "Address found for interface") \ -_(SESSION_CONNECT_FAIL, -115, "Session failed to connect") +_(SESSION_CONNECT_FAIL, -115, "Session failed to connect") \ +_(ENTRY_ALREADY_EXISTS, -116, "Entry already exists") typedef enum { diff --git a/src/vnet/ethernet/arp.c b/src/vnet/ethernet/arp.c index 711b7867fab..09ecd0ccebc 100644 --- a/src/vnet/ethernet/arp.c +++ b/src/vnet/ethernet/arp.c @@ -702,76 +702,59 @@ vnet_add_del_ip4_arp_change_event (vnet_main_t * vnm, { ethernet_arp_main_t *am = ðernet_arp_main; ip4_address_t *address = address_arg; - uword *p; + + /* Try to find an existing entry */ + u32 *first = (u32 *) hash_get (am->mac_changes_by_address, address->as_u32); + u32 *p = first; pending_resolution_t *mc; - void (*fp) (u32, u8 *) = data_callback; + while (p && *p != ~0) + { + mc = pool_elt_at_index (am->mac_changes, *p); + if (mc->node_index == node_index && mc->type_opaque == type_opaque + && mc->pid == pid) + break; + p = &mc->next_index; + } + int found = p && *p != ~0; if (is_add) { - pool_get (am->mac_changes, mc); + if (found) + return VNET_API_ERROR_ENTRY_ALREADY_EXISTS; - mc->next_index = ~0; - mc->node_index = node_index; - mc->type_opaque = type_opaque; - mc->data = data; - mc->data_callback = data_callback; - mc->pid = pid; + pool_get (am->mac_changes, mc); + *mc = (pending_resolution_t) + { + .next_index = ~0,.node_index = node_index,.type_opaque = + type_opaque,.data = data,.data_callback = data_callback,.pid = + pid,}; - p = hash_get (am->mac_changes_by_address, address->as_u32); + /* Insert new resolution at the end of the list */ + u32 new_idx = mc - am->mac_changes; if (p) - { - /* Insert new resolution at the head of the list */ - mc->next_index = p[0]; - hash_unset (am->mac_changes_by_address, address->as_u32); - } - - hash_set (am->mac_changes_by_address, address->as_u32, - mc - am->mac_changes); - return 0; + p[0] = new_idx; + else + hash_set (am->mac_changes_by_address, address->as_u32, new_idx); } else { - u32 index; - pending_resolution_t *mc_last = 0; - - p = hash_get (am->mac_changes_by_address, address->as_u32); - if (p == 0) + if (!found) return VNET_API_ERROR_NO_SUCH_ENTRY; - index = p[0]; + /* Clients may need to clean up pool entries, too */ + void (*fp) (u32, u8 *) = data_callback; + if (fp) + (*fp) (mc->data, 0 /* no new mac addrs */ ); - while (index != (u32) ~ 0) - { - mc = pool_elt_at_index (am->mac_changes, index); - if (mc->node_index == node_index && - mc->type_opaque == type_opaque && mc->pid == pid) - { - /* Clients may need to clean up pool entries, too */ - if (fp) - (*fp) (mc->data, 0 /* no new mac addrs */ ); - if (index == p[0]) - { - hash_unset (am->mac_changes_by_address, address->as_u32); - if (mc->next_index != ~0) - hash_set (am->mac_changes_by_address, address->as_u32, - mc->next_index); - pool_put (am->mac_changes, mc); - return 0; - } - else - { - ASSERT (mc_last); - mc_last->next_index = mc->next_index; - pool_put (am->mac_changes, mc); - return 0; - } - } - mc_last = mc; - index = mc->next_index; - } + /* Remove the entry from the list and delete the entry */ + *p = mc->next_index; + pool_put (am->mac_changes, mc); - return VNET_API_ERROR_NO_SUCH_ENTRY; + /* Remove from hash if we deleted the last entry */ + if (*p == ~0 && p == first) + hash_unset (am->mac_changes_by_address, address->as_u32); } + return 0; } /* Either we drop the packet or we send a reply to the sender. */ diff --git a/src/vpp/api/api.c b/src/vpp/api/api.c index f169d7fcfda..14ccd8648b4 100644 --- a/src/vpp/api/api.c +++ b/src/vpp/api/api.c @@ -1574,6 +1574,14 @@ vl_api_want_ip4_arp_events_t_handler (vl_api_want_ip4_arp_events_t * mp) if (mp->enable_disable) { + rv = vnet_add_del_ip4_arp_change_event + (vnm, arp_change_data_callback, + mp->pid, &mp->address /* addr, in net byte order */ , + vpe_resolver_process_node.index, + IP4_ARP_EVENT, event - am->arp_events, 1 /* is_add */ ); + + if (rv) + goto out; pool_get (am->arp_events, event); memset (event, 0, sizeof (*event)); @@ -1584,12 +1592,6 @@ vl_api_want_ip4_arp_events_t_handler (vl_api_want_ip4_arp_events_t * mp) event->pid = mp->pid; if (mp->address == 0) event->mac_ip = 1; - - rv = vnet_add_del_ip4_arp_change_event - (vnm, arp_change_data_callback, - mp->pid, &mp->address /* addr, in net byte order */ , - vpe_resolver_process_node.index, - IP4_ARP_EVENT, event - am->arp_events, 1 /* is_add */ ); } else { @@ -1599,6 +1601,7 @@ vl_api_want_ip4_arp_events_t_handler (vl_api_want_ip4_arp_events_t * mp) vpe_resolver_process_node.index, IP4_ARP_EVENT, ~0 /* pool index */ , 0 /* is_add */ ); } +out: REPLY_MACRO (VL_API_WANT_IP4_ARP_EVENTS_REPLY); } -- 2.16.6