ARP/ND/API:fix arp reg + nd no identical regs

fixed arp registration without allocating the event
added protection against identical ND registrations

Change-Id: I4e2db4913b35d895d8686ada1f0818920f276ad6
Signed-off-by: Eyal Bari <ebari@cisco.com>

diff --git a/src/vnet/ip/ip6_neighbor.c b/src/vnet/ip/ip6_neighbor.c
index 3d48c40..b1a0354 100644 (file)
--- a/src/vnet/ip/ip6_neighbor.c
+++ b/src/vnet/ip/ip6_neighbor.c
@@ -3910,76 +3910,59 @@ vnet_add_del_ip6_nd_change_event (vnet_main_t * vnm,
 {
   ip6_neighbor_main_t *nm = &ip6_neighbor_main;
   ip6_address_t *address = address_arg;
-  uword *p;
+
+  /* Try to find an existing entry */
+  u32 *first = (u32 *) mhash_get (&nm->mac_changes_by_address, address);
+  u32 *p = first;
   pending_resolution_t *mc;
-  void (*fp) (u32, u8 *) = data_callback;
+  while (p && *p != ~0)
+    {
+      mc = pool_elt_at_index (nm->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 (nm->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 (nm->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 = mhash_get (&nm->mac_changes_by_address, address);
+      /* Insert new resolution at the end of the list */
+      u32 new_idx = mc - nm->mac_changes;
       if (p)
-       {
-         /* Insert new resolution at the head of the list */
-         mc->next_index = p[0];
-         mhash_unset (&nm->mac_changes_by_address, address, 0);
-       }
-
-      mhash_set (&nm->mac_changes_by_address, address,
-                mc - nm->mac_changes, 0);
-      return 0;
+       p[0] = new_idx;
+      else
+       mhash_set (&nm->mac_changes_by_address, address, new_idx, 0);
     }
   else
     {
-      u32 index;
-      pending_resolution_t *mc_last = 0;
-
-      p = mhash_get (&nm->mac_changes_by_address, address);
-      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 (nm->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])
-               {
-                 mhash_unset (&nm->mac_changes_by_address, address, 0);
-                 if (mc->next_index != ~0)
-                   mhash_set (&nm->mac_changes_by_address, address,
-                              mc->next_index, 0);
-                 pool_put (nm->mac_changes, mc);
-                 return 0;
-               }
-             else
-               {
-                 ASSERT (mc_last);
-                 mc_last->next_index = mc->next_index;
-                 pool_put (nm->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 (nm->mac_changes, mc);
 
-      return VNET_API_ERROR_NO_SUCH_ENTRY;
+      /* Remove from hash if we deleted the last entry */
+      if (*p == ~0 && p == first)
+       mhash_unset (&nm->mac_changes_by_address, address, 0);
     }
+  return 0;
 }
 
 int

diff --git a/src/vpp/api/api.c b/src/vpp/api/api.c
index 09ae8b8..22410fc 100644 (file)
--- a/src/vpp/api/api.c
+++ b/src/vpp/api/api.c
@@ -199,7 +199,7 @@ int ip4_add_del_route_t_handler (vl_api_ip_add_del_route_t * mp);
 
 int ip6_add_del_route_t_handler (vl_api_ip_add_del_route_t * mp);
 
-void
+static void
 handle_ip4_arp_event (u32 pool_index)
 {
   vpe_api_main_t *vam = &vpe_api_main;
@@ -1571,11 +1571,12 @@ vl_api_want_ip4_arp_events_t_handler (vl_api_want_ip4_arp_events_t * mp)
   vpe_api_main_t *am = &vpe_api_main;
   vnet_main_t *vnm = vnet_get_main ();
   vl_api_want_ip4_arp_events_reply_t *rmp;
-  vl_api_ip4_arp_event_t *event;
   int rv;
 
   if (mp->enable_disable)
     {
+      vl_api_ip4_arp_event_t *event;
+      pool_get (am->arp_events, event);
       rv = vnet_add_del_ip4_arp_change_event
        (vnm, arp_change_data_callback,
         mp->pid, &mp->address /* addr, in net byte order */ ,
@@ -1583,8 +1584,10 @@ vl_api_want_ip4_arp_events_t_handler (vl_api_want_ip4_arp_events_t * mp)
         IP4_ARP_EVENT, event - am->arp_events, 1 /* is_add */ );
 
       if (rv)
-       goto out;
-      pool_get (am->arp_events, event);
+       {
+         pool_put (am->arp_events, event);
+         goto out;
+       }
       memset (event, 0, sizeof (*event));
 
       event->_vl_msg_id = ntohs (VL_API_IP4_ARP_EVENT);
@@ -1613,12 +1616,24 @@ vl_api_want_ip6_nd_events_t_handler (vl_api_want_ip6_nd_events_t * mp)
   vpe_api_main_t *am = &vpe_api_main;
   vnet_main_t *vnm = vnet_get_main ();
   vl_api_want_ip6_nd_events_reply_t *rmp;
-  vl_api_ip6_nd_event_t *event;
   int rv;
 
   if (mp->enable_disable)
     {
+      vl_api_ip6_nd_event_t *event;
       pool_get (am->nd_events, event);
+
+      rv = vnet_add_del_ip6_nd_change_event
+       (vnm, nd_change_data_callback,
+        mp->pid, mp->address /* addr, in net byte order */ ,
+        vpe_resolver_process_node.index,
+        IP6_ND_EVENT, event - am->nd_events, 1 /* is_add */ );
+
+      if (rv)
+       {
+         pool_put (am->nd_events, event);
+         goto out;
+       }
       memset (event, 0, sizeof (*event));
 
       event->_vl_msg_id = ntohs (VL_API_IP6_ND_EVENT);
@@ -1629,11 +1644,6 @@ vl_api_want_ip6_nd_events_t_handler (vl_api_want_ip6_nd_events_t * mp)
       if (ip6_address_is_zero ((ip6_address_t *) mp->address))
        event->mac_ip = 1;
 
-      rv = vnet_add_del_ip6_nd_change_event
-       (vnm, nd_change_data_callback,
-        mp->pid, mp->address /* addr, in net byte order */ ,
-        vpe_resolver_process_node.index,
-        IP6_ND_EVENT, event - am->nd_events, 1 /* is_add */ );
     }
   else
     {
@@ -1643,6 +1653,7 @@ vl_api_want_ip6_nd_events_t_handler (vl_api_want_ip6_nd_events_t * mp)
         vpe_resolver_process_node.index,
         IP6_ND_EVENT, ~0 /* pool index */ , 0 /* is_add */ );
     }
+out:
   REPLY_MACRO (VL_API_WANT_IP6_ND_EVENTS_REPLY);
 }