utils: cleanup

- Simplify linked list search - code was unnecessarily complex for no benefit
- Refactored parse / kick logic to simplify functions

[cleanup commit message]
Signed-off-by: Nick Hainke <vincent@systemli.org>

src/include/datastorage.h

@@ -222,16 +222,8 @@ extern pthread_mutex_t probe_array_mutex;
 #define MAX_INTERFACE_NAME 64
 
 // ---------------- Structs ----------------
-// Testing only: Removes the ability to find clients via secondary search, hence replicates
-// the pre-optimisation behaviour of only scanning the BSSID+MAC orderd list
-//#define DAWN_CLIENT_SCAN_BC_ONLY
-
 typedef struct client_s {
     struct client_s* next_entry_bc;
-    struct client_s* next_skip_entry_bc;
-#ifndef DAWN_CLIENT_SCAN_BC_ONLY
-    struct client_s* next_entry_c;
-#endif
     struct dawn_mac bssid_addr;
     struct dawn_mac client_addr;
     char signature[SIGNATURE_LEN]; // TODO: Never evaluated?
@@ -317,6 +309,8 @@ probe_entry* probe_array_update_rcpi_rsni(struct dawn_mac client_addr, struct da
 
 void remove_old_client_entries(time_t current_time, long long int threshold);
 
+client* client_array_update_entry(client* entry, time_t expiry);
+
 client *insert_client_to_array(client *entry, time_t expiry);
 
 int kick_clients(struct dawn_mac bssid, uint32_t id);
@@ -325,18 +319,16 @@ void update_iw_info(struct dawn_mac bssid);
 
 client** client_find_first_bc_entry(struct dawn_mac bssid_mac, struct dawn_mac client_mac, int do_client);
 
-void client_array_insert(client *entry, client ** insert_pos);
-
 client *client_array_get_client(const struct dawn_mac client_addr);
 
+client *client_array_delete_bc(struct dawn_mac bssid_mac, struct dawn_mac client_mac);
+
 client *client_array_delete(client *entry, int unlink_only);
 
 void print_client_array();
 
 void print_client_entry(int level, client *entry);
 
-int is_connected_somehwere(struct dawn_mac client_addr);
-
 ap *insert_to_ap_array(ap *entry, time_t expiry);
 
 void remove_old_ap_entries(time_t current_time, long long int threshold);

src/include/msghandler.h

@@ -20,7 +20,7 @@ probe_entry *parse_to_probe_req(struct blob_attr* msg);
  * @param id - ubus id.
  * @return
  */
-int parse_to_clients(struct blob_attr* msg, int do_kick, uint32_t id);
+int parse_to_clients(struct blob_attr* msg);
 
 /**
  * Parse to hostapd notify.

src/storage/datastorage.c

@@ -19,7 +19,7 @@ struct local_config_s local_config;
 
 #define MAC2STR(a) (a)[0], (a)[1], (a)[2], (a)[3], (a)[4], (a)[5]
 
-static int is_connected(struct dawn_mac bssid_mac, struct dawn_mac client_mac);
+static client* client_array_get_client_for_bssid(struct dawn_mac bssid_mac, struct dawn_mac client_mac);
 
 static int compare_station_count(ap* ap_entry_own, ap* ap_entry_to_compare, struct dawn_mac client_addr);
 
@@ -46,12 +46,7 @@ struct ap_s *ap_set = NULL;
 static int ap_entry_last = 0;
 pthread_mutex_t ap_array_mutex;
 
-#define DAWN_CLIENT_SKIP_RATIO 32
-static struct client_s* client_skip_set = NULL;
-static uint32_t client_skip_entry_last = 0;
 struct client_s* client_set_bc = NULL; // Ordered by BSSID + client MAC
-struct client_s* client_set_c = NULL; // Ordered by client MAC only
-static int client_entry_last = 0;
 pthread_mutex_t client_array_mutex;
 
 // TODO: How big does this get?
@@ -115,123 +110,41 @@ probe_entry* probe_array_find_first_entry(struct dawn_mac client_mac, struct daw
 
     return curr_node;
 }
+
 // Manage a list of client entries sorted by BSSID and client MAC
-static struct client_s** client_skip_array_find_first_entry(struct dawn_mac client_mac, struct dawn_mac bssid_mac, bool do_bssid)
-{
-    int lo = 0;
-    struct client_s** lo_ptr = &client_skip_set;
-    int hi = client_skip_entry_last;
-
-    dawnlog_debug_func("Entering...");
-
-    while (lo < hi) {
-        struct client_s** i = lo_ptr;
-        int scan_pos = lo;
-
-        // m is next test position of binary search
-        int m = (lo + hi) / 2;
-
-        // find entry with ordinal position m
-        while (scan_pos++ < m)
-        {
-            i = &((*i)->next_skip_entry_bc);
-        }
-
-        int this_cmp = mac_compare_bb((*i)->client_addr, client_mac);
-
-        if (this_cmp == 0 && do_bssid)
-            this_cmp = mac_compare_bb((*i)->bssid_addr, bssid_mac);
-
-        if (this_cmp < 0)
-        {
-            lo = m + 1;
-            lo_ptr = &((*i)->next_skip_entry_bc);
-        }
-        else
-        {
-            hi = m;
-        }
-    }
-
-    return lo_ptr;
-}
-
 client** client_find_first_bc_entry(struct dawn_mac bssid_mac, struct dawn_mac client_mac, int do_client)
 {
-    client ** lo_skip_ptr = &client_skip_set;
-    client ** lo_ptr = &client_set_bc;
-
     dawnlog_debug_func("Entering...");
 
-    while ((*lo_skip_ptr != NULL))
+    client** ret = &client_set_bc;
+
+    while ((*ret != NULL))
     {
-        int this_cmp = mac_compare_bb(((*lo_skip_ptr))->bssid_addr, bssid_mac);
+        int this_cmp = mac_compare_bb((*ret)->bssid_addr, bssid_mac);
 
         if (this_cmp == 0 && do_client)
-            this_cmp = mac_compare_bb(((*lo_skip_ptr))->client_addr, client_mac);
+            this_cmp = mac_compare_bb((*ret)->client_addr, client_mac);
 
         if (this_cmp >= 0)
             break;
 
-        lo_ptr = &((*lo_skip_ptr)->next_entry_bc);
+        ret = &((*ret)->next_entry_bc);
-        lo_skip_ptr = &((*lo_skip_ptr)->next_skip_entry_bc);
     }
 
-    while ((*lo_ptr != NULL))
+    return ret;
-    {
-        int this_cmp = mac_compare_bb((*lo_ptr)->bssid_addr, bssid_mac);
-
-        if (this_cmp == 0 && do_client)
-            this_cmp = mac_compare_bb((*lo_ptr)->client_addr, client_mac);
-
-        if (this_cmp >= 0)
-            break;
-
-        lo_ptr = &((*lo_ptr)->next_entry_bc);
-    }
-
-    return lo_ptr;
 }
 
-#ifndef DAWN_CLIENT_SCAN_BC_ONLY
+static client* client_array_get_client_for_bssid(struct dawn_mac bssid_mac, struct dawn_mac client_mac) {
-// Manage a list of client entries srted by client MAC only
-static client** client_find_first_c_entry(struct dawn_mac client_mac)
-{
-    int lo = 0;
-    client** lo_ptr = &client_set_c;
-    int hi = client_entry_last;
-
     dawnlog_debug_func("Entering...");
 
-    while (lo < hi) {
+    client* i = *client_find_first_bc_entry(bssid_mac, client_mac, true);
-        client** i = lo_ptr;
-        int scan_pos = lo;
 
-        // m is next test position of binary search
+    if (i != NULL)
-        int m = (lo + hi) / 2;
+        if (!mac_is_equal_bb((i)->bssid_addr, bssid_mac) || !mac_is_equal_bb((i)->client_addr, client_mac))
+            i = NULL;
 
-        // find entry with ordinal position m
+    return i;
-        while (scan_pos++ < m)
-        {
-            i = &((*i)->next_entry_c);
-        }
-
-        int this_cmp = mac_compare_bb((*i)->client_addr, client_mac);
-
-        if (this_cmp < 0)
-        {
-            lo = m + 1;
-            lo_ptr = &((*i)->next_entry_c);
-        }
-        else
-        {
-            hi = m;
-        }
-    }
-
-    return lo_ptr;
 }
-#endif
 
 struct mac_entry_s* mac_find_entry(struct dawn_mac mac)
 {
@@ -335,12 +248,12 @@ static int compare_station_count(ap* ap_entry_own, ap* ap_entry_to_compare, stru
 
     int sta_count = ap_entry_own->station_count;
     int sta_count_to_compare = ap_entry_to_compare->station_count;
-    if (is_connected(ap_entry_own->bssid_addr, client_addr)) {
+    if (client_array_get_client_for_bssid(ap_entry_own->bssid_addr, client_addr)) {
         dawnlog_debug("Own is already connected! Decrease counter!\n");
         sta_count--;
     }
 
-    if (is_connected(ap_entry_to_compare->bssid_addr, client_addr)) {
+    if (client_array_get_client_for_bssid(ap_entry_to_compare->bssid_addr, client_addr)) {
         dawnlog_debug("Comparing station is already connected! Decrease counter!\n");
         sta_count_to_compare--;
     }
@@ -620,6 +533,7 @@ int kick_clients(struct dawn_mac bssid_mac, uint32_t id) {
 
                             // don't delete clients in a row. use update function again...
                             // -> chan_util update, ...
+                            //FIXME: Why / 4?
                             add_client_update_timer(timeout_config.update_client * 1000 / 4);
                             break;
                         }
@@ -690,135 +604,29 @@ void update_iw_info(struct dawn_mac bssid_mac) {
     pthread_mutex_unlock(&client_array_mutex);
 }
 
-int is_connected_somehwere(struct dawn_mac client_addr) {
+client *client_array_get_client(const struct dawn_mac client_addr)
-    int found_in_array = 0;
+{
-
     dawnlog_debug_func("Entering...");
 
-#ifndef DAWN_CLIENT_SCAN_BC_ONLY
-    client* i = *client_find_first_c_entry(client_addr);
-#else
-    client* i = client_set_bc;
-    while (i != NULL && !mac_is_equal_bb(client_addr, i->client_addr))
-    {
-        i = i->next_entry_bc;
-    }
-#endif
-
-    if (i != NULL && mac_is_equal_bb(client_addr, i->client_addr))
-    {
-        found_in_array = 1;
-    }
-
-    return found_in_array;
-}
-
-static int is_connected(struct dawn_mac bssid_mac, struct dawn_mac client_mac) {
-    int found_in_array = 0;
-
-    dawnlog_debug_func("Entering...");
-
-    client** i = client_find_first_bc_entry(bssid_mac, client_mac, true);
-
-    if (*i != NULL && mac_is_equal_bb((*i)->bssid_addr, bssid_mac) && mac_is_equal_bb((*i)->client_addr, client_mac))
-        found_in_array = 1;
-
-    return found_in_array;
-}
-
-static struct client_s* insert_to_client_bc_skip_array(struct client_s* entry) {
-    dawnlog_debug_func("Entering...");
-
-
-    struct client_s** insert_pos = client_skip_array_find_first_entry(entry->client_addr, entry->bssid_addr, true);
-
-    entry->next_skip_entry_bc = *insert_pos;
-    *insert_pos = entry;
-    client_skip_entry_last++;
-
-    return entry;
-}
-
-void client_array_insert(client *entry, client** insert_pos) {
-    dawnlog_debug_func("Entering...");
-
-    // Passed insert_pos is where to insert in bc set
-    if (insert_pos == NULL)
-        insert_pos = client_find_first_bc_entry(entry->bssid_addr, entry->client_addr, true);
-    entry->next_entry_bc = *insert_pos;
-    *insert_pos = entry;
-
-#ifndef DAWN_CLIENT_SCAN_BC_ONLY
-    insert_pos = client_find_first_c_entry(entry->client_addr);
-    entry->next_entry_c = *insert_pos;
-    *insert_pos = entry;
-#endif
-
-    client_entry_last++;
-
-    // Try to keep skip list density stable
-    if ((client_entry_last / DAWN_CLIENT_SKIP_RATIO) > client_skip_entry_last)
-    {
-        entry->next_skip_entry_bc = NULL;
-        insert_to_client_bc_skip_array(entry);
-    }
-}
-
-client *client_array_get_client(const struct dawn_mac client_addr) {
-    dawnlog_debug_func("Entering...");
-
-    //pthread_mutex_lock(&client_array_mutex);
-
-#ifndef DAWN_CLIENT_SCAN_BC_ONLY
-    client* ret = *client_find_first_c_entry(client_addr);
-#else
     client* ret = client_set_bc;
     while (ret != NULL && !mac_is_equal_bb(client_addr, ret->client_addr))
     {
         ret = ret->next_entry_bc;
     }
-#endif
-
-    if (ret != NULL && !mac_is_equal_bb(client_addr, ret->client_addr))
-        ret = NULL;
-
-    //pthread_mutex_unlock(&client_array_mutex);
 
     return ret;
 }
 
 static client* client_array_unlink_entry(client** ref_bc, int unlink_only)
 {
-    client* entry = *ref_bc; // Both ref_bc and ref_c point to the entry we're deleting
-
     dawnlog_debug_func("Entering...");
+    client* entry = *ref_bc;
 
-    for (struct client_s** s = &client_skip_set; *s != NULL; s = &((*s)->next_skip_entry_bc)) {
-        if (*s == entry) {
-            *s = (*s)->next_skip_entry_bc;
-
-            client_skip_entry_last--;
-            break;
-        }
-    }
-
-    //  Accident of history that we always pass in the _bc ref, so need to find _c ref
-#ifndef DAWN_CLIENT_SCAN_BC_ONLY
-    client** ref_c = &client_set_c;
-    while ( *ref_c != NULL && *ref_c != entry)
-        ref_c = &((*ref_c)->next_entry_c);
-
-    *ref_c = entry->next_entry_c;
-#endif
     *ref_bc = entry->next_entry_bc;
-    client_entry_last--;
 
     if (unlink_only)
     {
         entry->next_entry_bc = NULL;
-#ifndef DAWN_CLIENT_SCAN_BC_ONLY
-        entry->next_entry_c = NULL;
-#endif
     }
     else
     {
@@ -829,13 +637,24 @@ static client* client_array_unlink_entry(client** ref_bc, int unlink_only)
     return entry;
 }
 
-client *client_array_delete(client *entry, int unlink_only) {
+client* client_array_delete_bc(struct dawn_mac bssid_mac, struct dawn_mac client_mac) {
-    client* ret = NULL;
-
-    client** ref_bc = NULL;
-
     dawnlog_debug_func("Entering...");
 
+    client* ret = NULL;
+    client** client_entry = client_find_first_bc_entry(bssid_mac, client_mac, true);
+
+    if (*client_entry && mac_is_equal_bb(bssid_mac, (*client_entry)->bssid_addr) && mac_is_equal_bb(client_mac, (*client_entry)->client_addr))
+        ret = client_array_unlink_entry(client_entry, false);
+
+    return ret;
+}
+
+client* client_array_delete(client* entry, int unlink_only) {
+    dawnlog_debug_func("Entering...");
+
+    client* ret = NULL;
+    client** ref_bc = NULL;
+
     // Bodyless for-loop: test done in control logic
     for (ref_bc = &client_set_bc; (*ref_bc != NULL) && (*ref_bc != entry); ref_bc = &((*ref_bc)->next_entry_bc));
 
@@ -1236,7 +1055,7 @@ void remove_old_probe_entries(time_t current_time, long long int threshold) {
     probe_entry** s = &(probe_set.first_probe_skip);
 
     while (*i != NULL ) {
-        if (((*i)->time < current_time - threshold) && !is_connected((*i)->bssid_addr, (*i)->client_addr)) {
+        if (((*i)->time < current_time - threshold) && !client_array_get_client_for_bssid((*i)->bssid_addr, (*i)->client_addr)) {
             probe_entry* victim = *i;
 
             *i = victim->next_probe;
@@ -1272,6 +1091,37 @@ void remove_old_ap_entries(time_t current_time, long long int threshold) {
     }
 }
 
+client* client_array_update_entry(client* entry, time_t expiry) {
+    dawnlog_debug_func("Entering...");
+
+    client* old_entry = NULL;
+    client** entry_pos = client_find_first_bc_entry(entry->bssid_addr, entry->client_addr, true);
+
+    entry->time = expiry;
+
+    if (*entry_pos && mac_compare_bb(entry->bssid_addr, (*entry_pos)->bssid_addr) == 0 && mac_compare_bb(entry->client_addr, (*entry_pos)->client_addr) == 0)
+    {
+        dawnlog_debug_func("Replacing entry...");
+        // Swap entries if same BSSID + MAC...
+        old_entry = *entry_pos;
+        entry->kick_count = old_entry->kick_count;  // TODO: Not sure we need to keep this...
+
+        entry->next_entry_bc = old_entry->next_entry_bc;
+        old_entry->next_entry_bc = NULL;
+        *entry_pos = entry;
+    }
+    else
+    {
+        dawnlog_debug_func("Adding entry...");
+        // ... or add if new
+        entry->kick_count = 0;
+
+        entry->next_entry_bc = *entry_pos;
+        *entry_pos = entry;
+    }
+
+    return old_entry;
+}
 
 client *insert_client_to_array(client *entry, time_t expiry) {
 client * ret = NULL;
@@ -1283,11 +1133,15 @@ client * ret = NULL;
     if (*client_tmp == NULL || !mac_is_equal_bb(entry->bssid_addr, (*client_tmp)->bssid_addr) || !mac_is_equal_bb(entry->client_addr, (*client_tmp)->client_addr)) {
         entry->kick_count = 0;
         entry->time = expiry;
-        client_array_insert(entry, client_tmp);
+
+        entry->next_entry_bc = *client_tmp;
+        *client_tmp = entry;
+
         ret = entry;
     }
-    else
+    else {
         (*client_tmp)->time = expiry;
+    }
 
     return ret;
 }
@@ -1415,7 +1269,7 @@ void print_client_array() {
     if (dawnlog_showing(DAWNLOG_DEBUG))
     {
         dawnlog_debug("--------Clients------\n");
-        dawnlog_debug("Client Entry Last: %d\n", client_entry_last);
+        //dawnlog_debug("Client Entry Last: %d\n", client_entry_last);
         for (client* i = client_set_bc; i != NULL; i = i->next_entry_bc) {
             print_client_entry(DAWNLOG_DEBUG, i);
         }

src/utils/msghandler.c

@@ -231,9 +231,7 @@ int handle_deauth_req(struct blob_attr* msg) {
 
     pthread_mutex_lock(&client_array_mutex);
 
-    client* client_entry = client_array_get_client(notify_req.client_addr);
+    client_array_delete_bc(notify_req.bssid_addr, notify_req.client_addr);
-    if (client_entry != NULL)
-        client_array_delete(client_entry, false);
 
     pthread_mutex_unlock(&client_array_mutex);
 
@@ -309,7 +307,7 @@ int handle_network_msg(char* msg) {
         }
     }
     else if (strncmp(method, "clients", 5) == 0) {
-        parse_to_clients(data_buf.head, 0, 0);
+        parse_to_clients(data_buf.head);
     }
     else if (strncmp(method, "deauth", 5) == 0) {
         dawnlog_debug("METHOD DEAUTH\n");
@@ -437,11 +435,12 @@ dump_client(struct blob_attr** tb, struct dawn_mac client_addr, const char* bssi
     }
 
     pthread_mutex_lock(&client_array_mutex);
-    // If entry was akraedy in list it won't be added, so free memorY
+    // If entry was already in list we get back the old entry, which needs to be freed
-    if (client_entry != insert_client_to_array(client_entry, time(0)))
+    client* prev_entry = client_array_update_entry(client_entry, time(0));
+    if (prev_entry)
     {
-        dawn_free(client_entry);
+        dawn_free(prev_entry);
-        client_entry = NULL;
+        prev_entry = NULL;
     }
 
     pthread_mutex_unlock(&client_array_mutex);
@@ -464,19 +463,13 @@ dump_client_table(struct blob_attr* head, int len, const char* bssid_addr, uint3
         blobmsg_parse(client_policy, __CLIENT_MAX, tb, blobmsg_data(attr), blobmsg_len(attr));
         //char* str = blobmsg_format_json_indent(attr, true, -1);
 
-        int tmp_int_mac[ETH_ALEN];
+        dump_client(tb, str2mac((char*)hdr->name), bssid_addr, freq, ht_supported, vht_supported);
-        struct dawn_mac tmp_mac;
-        sscanf((char*)hdr->name, MACSTR, STR2MAC(tmp_int_mac));
-        for (int i = 0; i < ETH_ALEN; ++i)
-            tmp_mac.u8[i] = (uint8_t)tmp_int_mac[i];
-
-        dump_client(tb, tmp_mac, bssid_addr, freq, ht_supported, vht_supported);
         station_count++;
     }
     return station_count;
 }
 
-int parse_to_clients(struct blob_attr* msg, int do_kick, uint32_t id) {
+int parse_to_clients(struct blob_attr* msg) {
     struct blob_attr* tb[__CLIENT_TABLE_MAX];
 
     dawnlog_debug_func("Entering...");
@@ -495,11 +488,14 @@ int parse_to_clients(struct blob_attr* msg, int do_kick, uint32_t id) {
 
     blobmsg_parse(client_table_policy, __CLIENT_TABLE_MAX, tb, blob_data(msg), blob_len(msg));
 
+    // Get clients
     if (tb[CLIENT_TABLE] && tb[CLIENT_TABLE_BSSID] && tb[CLIENT_TABLE_FREQ]) {
         int num_stations = 0;
         num_stations = dump_client_table(blobmsg_data(tb[CLIENT_TABLE]), blobmsg_data_len(tb[CLIENT_TABLE]),
             blobmsg_data(tb[CLIENT_TABLE_BSSID]), blobmsg_get_u32(tb[CLIENT_TABLE_FREQ]),
             blobmsg_get_u8(tb[CLIENT_TABLE_HT]), blobmsg_get_u8(tb[CLIENT_TABLE_VHT]));
+
+        // Get AP
         ap *ap_entry = dawn_malloc(sizeof(struct ap_s));
         hwaddr_aton(blobmsg_data(tb[CLIENT_TABLE_BSSID]), ap_entry->bssid_addr.u8);
         ap_entry->freq = blobmsg_get_u32(tb[CLIENT_TABLE_FREQ]);
@@ -579,11 +575,6 @@ int parse_to_clients(struct blob_attr* msg, int do_kick, uint32_t id) {
         }
 
         insert_to_ap_array(ap_entry, time(0));
-
-        if (do_kick && dawn_metric.kicking) {
-            update_iw_info(ap_entry->bssid_addr);
-            kick_clients(ap_entry->bssid_addr, id);
-        }
     }
     return 0;
 }

src/utils/ubus.c

@@ -218,6 +218,8 @@ static probe_entry* parse_to_beacon_rep(struct blob_attr *msg);
 
 void ubus_set_nr();
 
+
+/*** CODE START ***/
 void add_client_update_timer(time_t time) {
     uloop_timeout_set(&client_timer, time);
 }
@@ -699,7 +701,7 @@ int dawn_init_ubus(const char *ubus_socket, const char *hostapd_dir) {
     uloop_add_data_cbs();
 
     // get clients
-    uloop_timeout_add(&client_timer);  // callback = update_clients
+    uloop_timeout_add(&client_timer);  // callback = update_client_nr
 
     uloop_timeout_add(&channel_utilization_timer);  // callback = update_channel_utilization
 
@@ -800,8 +802,8 @@ static void ubus_get_clients_cb(struct ubus_request *req, int type, struct blob_
     blobmsg_add_string(&b, "hostname", entry->hostname);
 
     send_blob_attr_via_network(b.head, "clients");
-    // TODO: Have we just bit-packed data to send to something locally to unpack it again?  Performance / scalability?
+
-    parse_to_clients(b.head, 1, req->peer);
+    parse_to_clients(b.head);
 
     print_client_array();
     print_ap_array();
@@ -811,6 +813,13 @@ static void ubus_get_clients_cb(struct ubus_request *req, int type, struct blob_
 
     blob_buf_free(&b);
     dawn_unregmem(&b);
+
+    if (dawn_metric.kicking) {
+        //FIXME: Should we do this?  Does it mix up STA and AP RSSI values, and does that matter?
+        update_iw_info(entry->bssid_addr);
+
+        kick_clients(entry->bssid_addr, req->peer);
+    }
 }
 
 static int ubus_get_clients() {