[PATCH 11/16] hostapd: MLO: add support to remove the link before removing interface
Benjamin Berg
benjamin at sipsolutions.net
Mon Mar 18 05:55:25 PDT 2024
Hi,
On Wed, 2024-03-06 at 23:09 +0530, Aditya Kumar Singh wrote:
> Currently whenever if_remove() is called, whole interface is deleted.
> In MLO, all partner BSS uses the same driver private hence removing the
> interface when only one the link goes down should be avoided.
>
> Add helper function to remove link first whenever if_remove() is called.
> Later while handling it, if number of links active goes to 0, then the
> if_remove() would be called in order to clean up the interface.
>
> This helper would be used later when co-hosted MLD support is added as well
> later during ML reconfiguration support.
>
> Signed-off-by: Aditya Kumar Singh <quic_adisi at quicinc.com>
> ---
> src/ap/ap_drv_ops.c | 18 +++++++++++++
> src/ap/ap_drv_ops.h | 3 +++
> src/drivers/driver.h | 14 ++++++++++
> src/drivers/driver_nl80211.c | 50
> ++++++++++++++++++++++++++++++++++++
> src/drivers/driver_nl80211.h | 1 +
> 5 files changed, 86 insertions(+)
>
> diff --git a/src/ap/ap_drv_ops.c b/src/ap/ap_drv_ops.c
> index a6f53fd8cbb1..5dfcdac3a137 100644
> --- a/src/ap/ap_drv_ops.c
> +++ b/src/ap/ap_drv_ops.c
> @@ -571,6 +571,18 @@ int hostapd_if_add(struct hostapd_data *hapd,
> enum wpa_driver_if_type type,
> bridge, use_existing, 1);
> }
>
> +#ifdef CONFIG_IEEE80211BE
> +int hostapd_if_link_remove(struct hostapd_data *hapd, enum
> wpa_driver_if_type type,
> + const char *ifname, u8 link_id)
> +{
> + if (hapd->driver == NULL || hapd->drv_priv == NULL ||
> + hapd->driver->if_link_remove == NULL)
> + return -1;
> +
> + return hapd->driver->if_link_remove(hapd->drv_priv, type,
> ifname,
> + hapd->mld_link_id);
> +}
> +#endif /* CONFIG_IEEE80211BE */
>
> int hostapd_if_remove(struct hostapd_data *hapd, enum
> wpa_driver_if_type type,
> const char *ifname)
> @@ -578,6 +590,12 @@ int hostapd_if_remove(struct hostapd_data *hapd,
> enum wpa_driver_if_type type,
> if (hapd->driver == NULL || hapd->drv_priv == NULL ||
> hapd->driver->if_remove == NULL)
> return -1;
> +
> +#ifdef CONFIG_IEEE80211BE
> + if (hapd->conf->mld_ap)
> + return hostapd_if_link_remove(hapd, type, ifname,
> hapd->mld_link_id);
> +#endif /* CONFIG_IEEE80211BE */
> +
> return hapd->driver->if_remove(hapd->drv_priv, type,
> ifname);
> }
>
> diff --git a/src/ap/ap_drv_ops.h b/src/ap/ap_drv_ops.h
> index b3a96447947a..9c74ef579b2f 100644
> --- a/src/ap/ap_drv_ops.h
> +++ b/src/ap/ap_drv_ops.h
> @@ -458,6 +458,9 @@ static inline int hostapd_drv_link_add(struct
> hostapd_data *hapd,
> return hapd->driver->link_add(hapd->drv_priv, link_id, addr,
> hapd);
>
> }
> +
> +int hostapd_if_link_remove(struct hostapd_data *hapd, enum
> wpa_driver_if_type type,
> + const char *ifname, u8 link_id);
> #endif /* CONFIG_IEEE80211BE */
>
> #endif /* AP_DRV_OPS */
> diff --git a/src/drivers/driver.h b/src/drivers/driver.h
> index 8ffe487ae1c7..6616397fb5d9 100644
> --- a/src/drivers/driver.h
> +++ b/src/drivers/driver.h
> @@ -5142,6 +5142,20 @@ struct wpa_driver_ops {
> */
> int (*link_add)(void *priv, u8 link_id, const u8 *addr, void
> *bss_ctx);
>
> +#ifdef CONFIG_IEEE80211BE
> + /**
> + * if_link_remove - Remove a link alone from virtual
> interface
> + * @priv: Private driver interface data
> + * @type: Interface type
> + * @ifname: Interface name of the virtual interface from
> where link is
> + * to be removed
> + * @link_id: Valid link ID to remove
> + * Returns: 0 on success, -1 on failure
> + */
> + int (*if_link_remove)(void *priv, enum wpa_driver_if_type
> type,
> + const char *ifname, s8 link_id);
> +#endif /* CONFIG_IEEE80211BE */
> +
> #ifdef CONFIG_TESTING_OPTIONS
> int (*register_frame)(void *priv, u16 type,
> const u8 *match, size_t match_len,
> diff --git a/src/drivers/driver_nl80211.c
> b/src/drivers/driver_nl80211.c
> index ef9a513b663b..d5f7cc7d041c 100644
> --- a/src/drivers/driver_nl80211.c
> +++ b/src/drivers/driver_nl80211.c
> @@ -196,6 +196,7 @@ static int nl80211_leave_ibss(struct
> wpa_driver_nl80211_data *drv,
> static int i802_set_iface_flags(struct i802_bss *bss, int up);
> static int nl80211_set_param(void *priv, const char *param);
> static void nl80211_remove_links(struct i802_bss *bss);
> +static int nl80211_remove_link(struct i802_bss *bss, int link_id);
> #ifdef CONFIG_MESH
> static int nl80211_put_mesh_config(struct nl_msg *msg,
> struct wpa_driver_mesh_bss_params
> *params);
> @@ -10709,6 +10710,52 @@ static int driver_nl80211_if_remove(void
> *priv, enum wpa_driver_if_type type,
> return wpa_driver_nl80211_if_remove(bss, type, ifname);
> }
>
> +#ifdef CONFIG_IEEE80211BE
> +static int wpa_driver_nl80211_if_link_remove(struct i802_bss *bss,
> + enum wpa_driver_if_type
> type,
> + const char *ifname,
> + s8 link_id)
> +{
> + struct wpa_driver_nl80211_data *drv = bss->drv;
> +
> + wpa_printf(MSG_DEBUG, "nl80211: %s(type=%d ifname=%s
> links=0x%x) link_id=%d",
> + __func__, type, ifname, bss->valid_links,
> link_id);
> + wpa_printf(MSG_DEBUG, "nl80211: Teardown AP(%s) link %d",
> bss->ifname,
> + link_id);
> +
> + nl80211_remove_link(bss, link_id);
> +
> + bss->ctx = bss->flink->ctx;
> +
> + if (drv->first_bss == bss && !bss->valid_links)
> + drv->ctx = bss->ctx;
> +
> + if (!bss->valid_links) {
> + wpa_printf(MSG_DEBUG,
> + "nl80211: Only 1 link was there hence
> remove interface");
> + return wpa_driver_nl80211_if_remove(bss, type,
> ifname);
> + }
> +
> + return 0;
> +}
> +
> +static int driver_nl80211_if_link_remove(void *priv, enum
> wpa_driver_if_type type,
> + const char *ifname, s8
> link_id)
> +{
> + struct i802_bss *bss = priv;
> +
> + if (link_id < 0 || link_id >= MAX_NUM_MLD_LINKS)
> + return -1;
> +
> + if (type != WPA_IF_AP_BSS)
> + return -1;
> +
> + if (!(bss->valid_links & BIT(link_id)))
> + return -1;
> +
Maybe use nl80211_link_valid here?
> + return wpa_driver_nl80211_if_link_remove(bss, type, ifname, link_id);
> +}
> +#endif /* CONFIG_IEEE80211BE */
>
> static int driver_nl80211_send_mlme(void *priv, const u8 *data,
> size_t data_len, int noack,
> @@ -14038,6 +14085,9 @@ const struct wpa_driver_ops wpa_driver_nl80211_ops = {
> #endif /* CONFIG_DPP */
> .get_sta_mlo_info = nl80211_get_sta_mlo_info,
> .link_add = nl80211_link_add,
> +#ifdef CONFIG_IEEE80211BE
> + .if_link_remove = driver_nl80211_if_link_remove,
> +#endif /* CONFIG_IEEE80211BE */
> #ifdef CONFIG_TESTING_OPTIONS
> .register_frame = testing_nl80211_register_frame,
> .radio_disable = testing_nl80211_radio_disable,
> diff --git a/src/drivers/driver_nl80211.h b/src/drivers/driver_nl80211.h
> index 03d3c333b3d1..3e5a53452f00 100644
> --- a/src/drivers/driver_nl80211.h
> +++ b/src/drivers/driver_nl80211.h
> @@ -352,6 +352,7 @@ const char * nl80211_iftype_str(enum nl80211_iftype mode);
>
> void nl80211_restore_ap_mode(struct i802_bss *bss);
> struct i802_link * nl80211_get_link(struct i802_bss *bss, s8 link_id);
> +int nl80211_is_valid_link(struct i802_bss *bss, s8 link_id);
>
> static inline bool nl80211_link_valid(u16 links, s8 link_id)
> {
This looks like a left-over after a merge conflict or so. I kind of
like the nl80211_is_valid_link name more though.
Benjamin
More information about the Hostap
mailing list