Currently, whenever a new BSS is created, if it is an EHT then it is tied to a corresponding MLD structure. If structure does not exist already, a new one is created and tied to it. Accordingly link ID is assigned as well. However, when BSS is deleted, the MLD structure is not freed and when the next time it is again created, it increments the link ID further and gets a wrong link ID. For example - 2 GHz SLO AP case. First ADD, link ID 0 would be assigned and mld wlan0 would be created. When REMOVE is issued, BSS would be deleted but mld wlan0 will not. When again ADD is issued, BSS will tie back to mld wlan0 but this time link ID will be incremented again and 1 would be assigned. Hence at subsequent REMOVE/ADD, link ID keeps on incrementing. Since link ID remains till lifetime of the BSS and MLD, the next link ID counter can not be just reset back to 0 when a BSS is deleted. Otherwise, in interleaved link enable/disable case, link ID would be changed. Hence to overcome this situation, add changes so that whenever a BSS is deleted, if the MLD is not referenced by any other existing BSS, delete the MLD structure itself. In order to know how many BSSes are referring a given MLD, introduce a new member refcount in MLD. If he value is 0 then it is safe to delete the MLD. Signed-off-by: Aditya Kumar Singh <quic_adisi@xxxxxxxxxxx> --- src/ap/hostapd.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++ src/ap/hostapd.h | 8 +++ 2 files changed, 140 insertions(+) diff --git a/src/ap/hostapd.c b/src/ap/hostapd.c index 38c4e253047d..a8ae0130fef6 100644 --- a/src/ap/hostapd.c +++ b/src/ap/hostapd.c @@ -73,6 +73,11 @@ static void hostapd_switch_color_timeout_handler(void *eloop_data, void *user_ctx); #endif /* CONFIG_IEEE80211AX */ +#ifdef CONFIG_IEEE80211BE +static void hostapd_mld_ref_inc(struct hostapd_mld *mld); +static void hostapd_mld_ref_dec(struct hostapd_mld *mld); +#endif /* CONFIG_IEEE80211BE */ + int hostapd_for_each_interface(struct hapd_interfaces *interfaces, int (*cb)(struct hostapd_iface *iface, void *ctx), void *ctx) @@ -2891,6 +2896,11 @@ void hostapd_interface_free(struct hostapd_iface *iface) for (j = 0; j < iface->num_bss; j++) { if (!iface->bss) break; +#ifdef CONFIG_IEEE80211BE + if (iface->bss[j]) + hostapd_mld_ref_dec(iface->bss[j]->mld); +#endif /* CONFIG_IEEE80211BE */ + wpa_printf(MSG_DEBUG, "%s: free hapd %p", __func__, iface->bss[j]); os_free(iface->bss[j]); @@ -2913,6 +2923,34 @@ struct hostapd_iface * hostapd_alloc_iface(void) } #ifdef CONFIG_IEEE80211BE +static void hostapd_mld_ref_inc(struct hostapd_mld *mld) +{ + if (!mld) + return; + + if (mld->refcount == HOSTAPD_MLD_MAX_REF_COUNT) { + wpa_printf(MSG_ERROR, "MLD %s: Ref count overflow", + mld->name); + return; + } + + mld->refcount++; +} + +static void hostapd_mld_ref_dec(struct hostapd_mld *mld) +{ + if (!mld) + return; + + if (!mld->refcount) { + wpa_printf(MSG_ERROR, "MLD %s: Ref count underflow", + mld->name); + return; + } + + mld->refcount--; +} + static void hostapd_bss_alloc_link_id(struct hostapd_data *hapd) { hapd->mld_link_id = hapd->mld->next_link_id++; @@ -2943,6 +2981,7 @@ static void hostapd_bss_setup_multi_link(struct hostapd_data *hapd, continue; hapd->mld = mld; + hostapd_mld_ref_inc(mld); hostapd_bss_alloc_link_id(hapd); break; } @@ -2957,12 +2996,14 @@ static void hostapd_bss_setup_multi_link(struct hostapd_data *hapd, os_strlcpy(mld->name, conf->iface, sizeof(conf->iface)); mld->next_link_id = 0; mld->num_links = 0; + mld->refcount = 0; mld->fbss = NULL; dl_list_init(&mld->links); wpa_printf(MSG_DEBUG, "MLD %s created", mld->name); hapd->mld = mld; + hostapd_mld_ref_inc(mld); hostapd_bss_alloc_link_id(hapd); all_mld = os_realloc_array(interfaces->mld, interfaces->mld_count + 1, @@ -2983,6 +3024,81 @@ fail: os_free(mld); hapd->mld = NULL; } + +static void hostapd_cleanup_unused_mlds(struct hapd_interfaces *interfaces) +{ + struct hostapd_mld *mld, **all_mld; + size_t i, j, num_mlds; + bool forced_remove, remove; + + if (!interfaces || !interfaces->mld) + return; + + num_mlds = interfaces->mld_count; + + for (i = 0; i < interfaces->mld_count; i++) { + mld = interfaces->mld[i]; + if (!mld) + continue; + + remove = false; + forced_remove = false; + + if (!mld->refcount) + remove = true; + + /* If mld is still being referenced but no. of interfaces + * are zero then it is safe to force delete it. Normally, + * this should not happen but even if it does, let us free + * the memory + */ + if (!remove && !interfaces->count) + forced_remove = true; + + if (!remove && !forced_remove) + continue; + + wpa_printf(MSG_DEBUG, "MLD %s: freed (forced: %d)", mld->name, + forced_remove); + os_free(mld); + interfaces->mld[i] = NULL; + num_mlds--; + } + + if (!num_mlds) { + interfaces->mld_count = 0; + os_free(interfaces->mld); + interfaces->mld = NULL; + return; + } + + all_mld = os_zalloc(num_mlds * sizeof(struct hostapd_mld *)); + if (!all_mld) { + wpa_printf(MSG_DEBUG, + "MLD: failed to re-allocate the MLDs. Expect issues"); + return; + } + + for (i = 0, j = 0; i < interfaces->mld_count; i++) { + mld = interfaces->mld[i]; + if (!mld) + continue; + + all_mld[j++] = mld; + } + + /* should not happen */ + if (j != num_mlds) { + wpa_printf(MSG_DEBUG, + "MLD: some error occurred while reallocating MLDs. Expect issues."); + os_free(all_mld); + return; + } + + os_free(interfaces->mld); + interfaces->mld = all_mld; + interfaces->mld_count = num_mlds; +} #endif /* CONFIG_IEEE80211BE */ /** @@ -3587,6 +3703,9 @@ int hostapd_add_iface(struct hapd_interfaces *interfaces, char *buf) __func__, hapd, hapd->conf->iface); hostapd_config_free_bss(hapd->conf); hapd->conf = NULL; +#ifdef CONFIG_IEEE80211BE + hostapd_mld_ref_dec(hapd->mld); +#endif /* CONFIG_IEEE80211BE */ os_free(hapd); return -1; } @@ -3678,6 +3797,9 @@ fail: hapd->conf->iface); hostapd_bss_link_deinit(hapd); hostapd_cleanup(hapd); +#ifdef CONFIG_IEEE80211BE + hostapd_mld_ref_dec(hapd->mld); +#endif /* CONFIG_IEEE80211BE */ os_free(hapd); hapd_iface->bss[i] = NULL; } @@ -3687,6 +3809,9 @@ fail: if (new_iface) { interfaces->count--; interfaces->iface[interfaces->count] = NULL; +#ifdef CONFIG_IEEE80211BE + hostapd_cleanup_unused_mlds(interfaces); +#endif /* CONFIG_IEEE80211BE */ } hostapd_cleanup_iface(hapd_iface); } @@ -3709,6 +3834,9 @@ static int hostapd_remove_bss(struct hostapd_iface *iface, unsigned int idx) __func__, hapd, hapd->conf->iface); hostapd_config_free_bss(hapd->conf); hapd->conf = NULL; +#ifdef CONFIG_IEEE80211BE + hostapd_mld_ref_dec(hapd->mld); +#endif /* CONFIG_IEEE80211BE */ os_free(hapd); iface->num_bss--; @@ -3751,6 +3879,10 @@ int hostapd_remove_iface(struct hapd_interfaces *interfaces, char *buf) k++; } interfaces->count--; +#ifdef CONFIG_IEEE80211BE + hostapd_cleanup_unused_mlds(interfaces); +#endif /* CONFIG_IEEE80211BE */ + return 0; } diff --git a/src/ap/hostapd.h b/src/ap/hostapd.h index ec090cbbaa85..27b158c51de9 100644 --- a/src/ap/hostapd.h +++ b/src/ap/hostapd.h @@ -506,10 +506,18 @@ struct hostapd_mld { u8 mld_addr[ETH_ALEN]; u8 next_link_id; u8 num_links; + /* No. of hostapd_data (hapd) referencing to this. num_links can't + * be used since num_links can go to 0 even when bss is disabled + * and when it is enabled back, the MLD should exist hence it can + * not be freed when it is 0. + */ + u8 refcount; struct hostapd_data *fbss; struct dl_list links; /* List HEAD of all affiliated links */ }; + +#define HOSTAPD_MLD_MAX_REF_COUNT 0xFF #endif /* CONFIG_IEEE80211BE */ /** -- 2.25.1 _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap