| Message ID | 20240306173947.2611965-12-quic_adisi@quicinc.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | hostapd: scale up MLO flows | expand |
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@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
On 3/18/24 18:25, Benjamin Berg wrote: ... >> +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? > Yeah can be used. Good point. ... >> 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 Oh yeah! Thanks for pointing it out. It can be removed. I'm surprised that for some reason the compilation did not flag this unused function prototype. > like the nl80211_is_valid_link name more though. May be later we can simply rename this function ;-p Hi Jouni, Will you fix these (minor changes) while applying or shall I re-spin a new version?
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; + + 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) {
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@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(+)