From fd1bd431ea9f10b968ff5251004fcc9e7a3fa523 Mon Sep 17 00:00:00 2001 From: Dhiren Vispute <86131363+dv-msft@users.noreply.github.com> Date: Fri, 3 Feb 2023 15:32:05 -0800 Subject: [PATCH] Incorporate post-merge feedback on PR#2004 (#2027) --- include/ebpf_result.h | 4 ++-- libs/execution_context/ebpf_maps.c | 30 +++++++++++++++--------------- libs/platform/ebpf_object.c | 29 ++++++++++++++--------------- 3 files changed, 31 insertions(+), 32 deletions(-) diff --git a/include/ebpf_result.h b/include/ebpf_result.h index ec64ec9ae1..e8f791e23d 100644 --- a/include/ebpf_result.h +++ b/include/ebpf_result.h @@ -113,8 +113,8 @@ extern "C" /// Operation timed out. EBPF_TIMEOUT, - /// Key is valid, but the object has been deleted. - EBPF_STALE_KEY, + /// ID is valid, but the object has been deleted. + EBPF_STALE_ID, } ebpf_result_t; #define EBPF_RESULT_COUNT (EBPF_INVALID_POINTER + 1) diff --git a/libs/execution_context/ebpf_maps.c b/libs/execution_context/ebpf_maps.c index 0966c99be1..6fa81558d2 100644 --- a/libs/execution_context/ebpf_maps.c +++ b/libs/execution_context/ebpf_maps.c @@ -679,10 +679,10 @@ _update_array_map_entry_with_handle( if (old_id) { // Release the reference on the old ID's id table entry. The object may have been already deleted, so an - // error return value of 'not found' is ok. + // error return value of 'stale id' is ok. result = ebpf_object_release_id_reference(old_id, value_type); - ebpf_assert(result == EBPF_SUCCESS || result == EBPF_STALE_KEY); - if (result == EBPF_STALE_KEY) { + ebpf_assert(result == EBPF_SUCCESS || result == EBPF_STALE_ID); + if (result == EBPF_STALE_ID) { result = EBPF_SUCCESS; } } @@ -747,10 +747,10 @@ _delete_array_map_entry_with_reference( ebpf_id_t id = *(ebpf_id_t*)entry; if (id) { - // The object may have been already deleted, so an error return value of 'stale key' is ok. + // The object may have been already deleted, so an error return value of 'stale id' is ok. result = ebpf_object_release_id_reference(id, value_type); - ebpf_assert(result == EBPF_SUCCESS || result == EBPF_STALE_KEY); - if (result == EBPF_STALE_KEY) { + ebpf_assert(result == EBPF_SUCCESS || result == EBPF_STALE_ID); + if (result == EBPF_STALE_ID) { result = EBPF_SUCCESS; } } @@ -903,10 +903,10 @@ _delete_object_hash_map(_In_ _Post_invalid_ ebpf_core_map_t* map) ebpf_id_t id = *(ebpf_id_t*)value; if (id) { - // The object may have been already deleted, so an error return value of 'stale key' is ok. + // The object may have been already deleted, so an error return value of 'stale id' is ok. result = ebpf_object_release_id_reference(id, EBPF_OBJECT_MAP); - ebpf_assert(result == EBPF_SUCCESS || result == EBPF_STALE_KEY); - if (result == EBPF_STALE_KEY) { + ebpf_assert(result == EBPF_SUCCESS || result == EBPF_STALE_ID); + if (result == EBPF_STALE_ID) { result = EBPF_SUCCESS; } } @@ -1404,10 +1404,10 @@ _update_hash_map_entry_with_handle( if (old_id) { // Release the reference on the old ID's id table entry. The object may already have been deleted, so an - // error return value of 'not found' is ok. + // error return value of 'stale id' is ok. result = ebpf_object_release_id_reference(old_id, value_type); - ebpf_assert(result == EBPF_SUCCESS || result == EBPF_STALE_KEY); - if (result == EBPF_STALE_KEY) { + ebpf_assert(result == EBPF_SUCCESS || result == EBPF_STALE_ID); + if (result == EBPF_STALE_ID) { result = EBPF_SUCCESS; } } @@ -1456,10 +1456,10 @@ _delete_map_hash_map_entry(_Inout_ ebpf_core_map_t* map, _In_ const uint8_t* key ebpf_id_t id = *(ebpf_id_t*)value; if (id) { - // The object may have been already deleted, so an error return value of 'stale key' is ok. + // The object may have been already deleted, so an error return value of 'stale id' is ok. result = ebpf_object_release_id_reference(id, EBPF_OBJECT_MAP); - ebpf_assert(result == EBPF_SUCCESS || result == EBPF_STALE_KEY); - if (result == EBPF_STALE_KEY) { + ebpf_assert(result == EBPF_SUCCESS || result == EBPF_STALE_ID); + if (result == EBPF_STALE_ID) { result = EBPF_SUCCESS; } } diff --git a/libs/platform/ebpf_object.c b/libs/platform/ebpf_object.c index 927c9e1ff5..729ba7c2c9 100644 --- a/libs/platform/ebpf_object.c +++ b/libs/platform/ebpf_object.c @@ -10,20 +10,19 @@ static ebpf_lock_t _ebpf_object_tracking_list_lock = {0}; /** * @brief Objects are allocated an entry in the the ID - * table when they are initialized. Along with a pointer to - * the object, each id table entry maintains its own ref-count - * that starts off at 1 when it is assigned to a new object. - * The entry ref-count indicates the number of other objects - * holding a reference to the corresponding object's id. On - * the destruction of an object, the object pointer in the - * corresponding id table entry is reset to NULL and the entry - * ref-count is also decremented. Note that the entry will - * continue to be considered 'in use' until all other objects - * are done with the associated object (they let go of their - * references to this entry). When the entry ref-count goes - * down to 0 _and_ the object pointer is NULL, it is eligible - * for re-use. Note that either of these events can occur - * first. + * table when they are initialized. Along with a pointer to the + * object, each id table entry maintains its own ref-count that + * starts off at 1 when it is assigned to a new object. The + * entry ref-count indicates the number of other objects + * holding a reference to the corresponding object's id. On the + * destruction of an object, the object pointer in the + * corresponding id table entry is reset to NULL and the entry + * ref-count is also decremented. Note that the entry will + * continue to be considered 'in use' until all other objects + * are done with the associated object (they let go of their + * references to this entry). When the entry ref-count goes + * down to 0 _and_ the object pointer is NULL, it is eligible + * for re-use. Note that either of these events can occur first. * * Map objects can have references due to one of the following: * 1) An open handle holds a reference on it. @@ -434,7 +433,7 @@ ebpf_object_acquire_id_reference(ebpf_id_t id, ebpf_object_type_t object_type) // The object at this entry has been deleted and all that remains are (the now stale) references to this entry // held by other objects. This is a non-reversible situation and there's no point in giving out references // for such entries, so deny with a suitable error code. - result = EBPF_STALE_KEY; + result = EBPF_STALE_ID; goto Done; }