From cbe8d65068c9c39ca6b1f7e116dac2744835f18c Mon Sep 17 00:00:00 2001 From: Neale Ranns Date: Fri, 27 Apr 2018 04:42:47 -0700 Subject: [PATCH] TAP memory leaks: 1 - use bit-map to re-use ID values and thus VLIB nodes 2 - free vrings 3 - free hw_address on HW interface delete (a HW * struct is memset on pool_get) 4 - free temporary node names during TX node setup Change-Id: Id114c8bb9c844fd4ceb02fbbeb4b511ecfeb61ce Signed-off-by: Neale Ranns --- src/vnet/devices/tap/tap.c | 38 +++++++++++++++----------------------- src/vnet/devices/tap/tap.h | 6 +++--- src/vnet/ethernet/interface.c | 1 - src/vnet/interface.c | 5 ++++- 4 files changed, 22 insertions(+), 28 deletions(-) diff --git a/src/vnet/devices/tap/tap.c b/src/vnet/devices/tap/tap.c index 7cffaafb1e2..2341bbbd140 100644 --- a/src/vnet/devices/tap/tap.c +++ b/src/vnet/devices/tap/tap.c @@ -76,6 +76,7 @@ open_netns_fd (char *netns) return fd; } +#define TAP_MAX_INSTANCE 1024 void tap_create_if (vlib_main_t * vm, tap_create_if_args_t * args) @@ -93,12 +94,10 @@ tap_create_if (vlib_main_t * vm, tap_create_if_args_t * args) struct vhost_memory *vhost_mem = 0; virtio_if_t *vif = 0; clib_error_t *err = 0; - uword *p; if (args->id != ~0) { - p = hash_get (tm->dev_instance_by_interface_id, args->id); - if (p) + if (clib_bitmap_get (tm->tap_ids, args->id)) { args->rv = VNET_API_ERROR_INVALID_INTERFACE; args->error = clib_error_return (0, "interface already exists"); @@ -107,22 +106,14 @@ tap_create_if (vlib_main_t * vm, tap_create_if_args_t * args) } else { - int tries = 1000; - while (--tries) - { - args->id = tm->last_used_interface_id++; - p = hash_get (tm->dev_instance_by_interface_id, args->id); - if (!p) - break; - } + args->id = clib_bitmap_first_clear (tm->tap_ids); + } - if (!tries) - { - args->rv = VNET_API_ERROR_UNSPECIFIED; - args->error = - clib_error_return (0, "cannot find free interface id"); - return; - } + if (args->id > TAP_MAX_INSTANCE) + { + args->rv = VNET_API_ERROR_UNSPECIFIED; + args->error = clib_error_return (0, "cannot find free interface id"); + return; } memset (&ifr, 0, sizeof (ifr)); @@ -382,8 +373,7 @@ tap_create_if (vlib_main_t * vm, tap_create_if_args_t * args) goto error; } - hash_set (tm->dev_instance_by_interface_id, vif->id, vif->dev_instance); - + tm->tap_ids = clib_bitmap_set (tm->tap_ids, vif->id, 1); sw = vnet_get_hw_sw_interface (vnm, vif->hw_if_index); vif->sw_if_index = sw->sw_if_index; args->sw_if_index = vif->sw_if_index; @@ -415,6 +405,7 @@ error: if (vif->fd != -1) close (vif->fd); vec_foreach_index (i, vif->vrings) virtio_vring_free (vm, vif, i); + vec_free (vif->vrings); memset (vif, 0, sizeof (virtio_if_t)); pool_put (vim->interfaces, vif); @@ -457,7 +448,7 @@ tap_delete_if (vlib_main_t * vm, u32 sw_if_index) vec_foreach_index (i, vif->vrings) virtio_vring_free (vm, vif, i); vec_free (vif->vrings); - hash_unset (tm->dev_instance_by_interface_id, vif->id); + tm->tap_ids = clib_bitmap_set (tm->tap_ids, vif->id, 0); clib_spinlock_free (&vif->lockp); memset (vif, 0, sizeof (*vif)); pool_put (mm->interfaces, vif); @@ -530,10 +521,11 @@ tap_init (vlib_main_t * vm) error = vlib_call_init_function (vm, vlib_log_init); if (error) return error; - tm->dev_instance_by_interface_id = hash_create (0, sizeof (uword)); + tm->log_default = vlib_log_register_class ("tap", 0); vlib_log_info (tm->log_default, "initialized"); - return 0; + + return NULL; } VLIB_INIT_FUNCTION (tap_init); diff --git a/src/vnet/devices/tap/tap.h b/src/vnet/devices/tap/tap.h index 6e46302770d..7aebade59c7 100644 --- a/src/vnet/devices/tap/tap.h +++ b/src/vnet/devices/tap/tap.h @@ -67,11 +67,11 @@ typedef struct typedef struct { - u32 last_used_interface_id; - uword *dev_instance_by_interface_id; - /* logging */ vlib_log_class_t log_default; + + /* bit-map of in-use IDs */ + uword *tap_ids; } tap_main_t; void tap_create_if (vlib_main_t * vm, tap_create_if_args_t * args); diff --git a/src/vnet/ethernet/interface.c b/src/vnet/ethernet/interface.c index 2ed20e15c24..174b3639f41 100644 --- a/src/vnet/ethernet/interface.c +++ b/src/vnet/ethernet/interface.c @@ -307,7 +307,6 @@ ethernet_register_interface (vnet_main_t * vnm, hi->max_l3_packet_bytes[VLIB_RX] = hi->max_l3_packet_bytes[VLIB_TX] = 9000; clib_memcpy (ei->address, address, sizeof (ei->address)); - vec_free (hi->hw_address); vec_add (hi->hw_address, address, sizeof (ei->address)); if (error) diff --git a/src/vnet/interface.c b/src/vnet/interface.c index 13ad65ee2db..d3ad896be12 100644 --- a/src/vnet/interface.c +++ b/src/vnet/interface.c @@ -717,7 +717,7 @@ vnet_register_interface (vnet_main_t * vnm, vnet_feature_config_main_t *fcm; vnet_config_main_t *cm; u32 hw_index, i; - char *tx_node_name, *output_node_name; + char *tx_node_name = NULL, *output_node_name = NULL; pool_get (im->hw_interfaces, hw); memset (hw, 0, sizeof (*hw)); @@ -907,6 +907,8 @@ no_output_nodes: VNET_INTERFACE_SET_FLAGS_HELPER_IS_CREATE); vnet_hw_interface_set_flags_helper (vnm, hw_index, /* flags */ 0, VNET_INTERFACE_SET_FLAGS_HELPER_IS_CREATE); + vec_free (tx_node_name); + vec_free (output_node_name); return hw_index; } @@ -973,6 +975,7 @@ vnet_delete_hw_interface (vnet_main_t * vnm, u32 hw_if_index) hash_unset_mem (im->hw_interface_by_name, hw->name); vec_free (hw->name); + vec_free (hw->hw_address); vec_free (hw->input_node_thread_index_by_queue); vec_free (hw->dq_runtime_index_by_queue); -- 2.16.6