diff mbox series

[09/50] AP: Allow starting multiple interfaces within single MLD

Message ID 20230215230904.933291-10-andrei.otcheretianski@intel.com
State Changes Requested
Headers show
Series Add basic MLO support for AP | expand

Commit Message

Andrei Otcheretianski Feb. 15, 2023, 11:08 p.m. UTC 
Add support for including multiple hostapd interfaces in
the same AP MLD, i.e., all using the same underlying driver
network interface.

To do so, when a new hostapd interface is added, if there is
already another interface using the same underlying network
interface, associate the new interface with the same private
data object, instead of creating a new one.

As some of the BSS's are non first BSS's, meaning that they reuse
the drv_priv of the initial BSS, make sure not to double free it.

Currently multiple BSS entries are not supported so always use bss[0]
for MLD.

Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
Signed-off-by: Ilan Peer <ilan.peer@intel.com>
---
 hostapd/main.c      | 84 +++++++++++++++++++++++++++++++++++++++++++++
 src/ap/ap_drv_ops.h | 12 +++++++
 src/ap/hostapd.c    | 39 +++++++++++++++++----
 src/ap/hostapd.h    |  2 ++
 4 files changed, 131 insertions(+), 6 deletions(-)

Comments

Rameshkumar Sundaram (QUIC) Feb. 17, 2023, 10:10 a.m. UTC | #1 
> -----Original Message-----
> From: Hostap <hostap-bounces@lists.infradead.org> On Behalf Of Andrei
> Otcheretianski
> Sent: Thursday, February 16, 2023 4:38 AM
> To: hostap@lists.infradead.org
> Cc: Andrei Otcheretianski <andrei.otcheretianski@intel.com>; Ilan Peer
> <ilan.peer@intel.com>
> Subject: [PATCH 09/50] AP: Allow starting multiple interfaces within single
> MLD
> 
> Add support for including multiple hostapd interfaces in the same AP MLD,
> i.e., all using the same underlying driver network interface.
> 
> To do so, when a new hostapd interface is added, if there is already another
> interface using the same underlying network interface, associate the new
> interface with the same private data object, instead of creating a new one.
> 
> As some of the BSS's are non first BSS's, meaning that they reuse the drv_priv
> of the initial BSS, make sure not to double free it.
> 
> Currently multiple BSS entries are not supported so always use bss[0] for
> MLD.
> 
> Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
> Signed-off-by: Ilan Peer <ilan.peer@intel.com>
> ---
>  hostapd/main.c      | 84
> +++++++++++++++++++++++++++++++++++++++++++++
>  src/ap/ap_drv_ops.h | 12 +++++++
>  src/ap/hostapd.c    | 39 +++++++++++++++++----
>  src/ap/hostapd.h    |  2 ++
>  4 files changed, 131 insertions(+), 6 deletions(-)
> 
> diff --git a/hostapd/main.c b/hostapd/main.c index ce2df81c4a..228ee44b1b
> 100644
> --- a/hostapd/main.c
> +++ b/hostapd/main.c
> @@ -164,6 +164,59 @@ static int hostapd_driver_init(struct hostapd_iface
> *iface)
>  		return -1;
>  	}
> 
> +#ifdef CONFIG_IEEE80211BE
> +		if (conf->mld_ap) {
> +			for (i = 0; i < iface->interfaces->count; i++) {
> +				struct hostapd_iface *h = iface->interfaces-
> >iface[i];
> +				struct hostapd_data *h_hapd = h->bss[0];
> +				struct hostapd_bss_config *hconf = h_hapd-
> >conf;
> +
> +				if (h == iface) {
> +					wpa_printf(MSG_ERROR, "Skip own
> iface");
> +					continue;
> +				}
> +
> +				if (!hconf->mld_ap || hconf->mld_id != conf-
> >mld_id) {
How about hconf->iface ? Should we check if its same across links of the MLD ?
> +					wpa_printf(MSG_ERROR,
> +						   "Skip non matching
> mld_id");
> +					continue;
> +				}
> +
> +				wpa_printf(MSG_DEBUG, "Found matching
> MLD iface");
> +				if (!h_hapd->drv_priv) {
> +					wpa_printf(MSG_ERROR,
> +						   "Matching MLD BSS not
> initialized yet");
> +					continue;
> +				}
> +
> +				hapd->drv_priv = h_hapd->drv_priv;
Having i802_bss common for all the links and belonging to only one drv, might have issue for cases of Multi vap/mbssid,
For ex.
1. get_bss_ifindex() will fail in cases where the ML bss (i802_bss) is not part of that drv but channel switch happens on that link.
2.In if_add, the hapd->iface->bss[0]->drv_priv(i802_bss)->drv might be pointing to first link's drv,  in some case of  multi vap MLD, like two 2+5 MLD, new bss (ML/Non-ML) might be pointing to first link BSS.
So far drv was tied to a link (hapd->iface), now well be having same drv for all links (all bss across links).
> +				/*
Andrei Otcheretianski Feb. 21, 2023, 9:21 a.m. UTC | #2 
> > +				if (!hconf->mld_ap || hconf->mld_id != conf-
> > >mld_id) {

> How about hconf->iface ? Should we check if its same across links of the MLD
> ?

Right. This would be needed if we have multiple MLD AP's over different interfaces that share the same mld_id.

> > +				hapd->drv_priv = h_hapd->drv_priv;
> Having i802_bss common for all the links and belonging to only one drv, might
> have issue for cases of Multi vap/mbssid, For ex.
> 1. get_bss_ifindex() will fail in cases where the ML bss (i802_bss) is not part
> of that drv but channel switch happens on that link.

As I mentioned in the cover letter, this is very initial and minimal support for MLD.
We didn't intend yet to support MBSSID and multi-vaps and channel switches - so probably there will be issues in these scenarios.
However I don't see any problem with extending it to properly support this in the future.
Channel switch notification includes both ifindex and link_id so it's not a problem to route it to the correct link/bss.
It's not possible to have an MLD AP that has different links using different interfaces.

> 2.In if_add, the hapd->iface->bss[0]->drv_priv(i802_bss)->drv might be
> pointing to first link's drv,  in some case of  multi vap MLD, like two 2+5 MLD,
> new bss (ML/Non-ML) might be pointing to first link BSS.
> So far drv was tied to a link (hapd->iface), now well be having same drv for all
> links (all bss across links).

This is correct, but I'm not sure I understand why it's a problem.. All the BSS's (hostapd_data) (MLD or not) that share the same interface (netdev interface) share the drv.
The addition for MLD is that now we can have multiple hostapd_iface structs (which represent a radio) which share the same netdev interface and thus they also share the drv and distinguished by mld_link_id which is passed in the relevant events. So all the notifications that have mld_link_id can be routed to a specific hostapd_iface and eventually to hapd.

Andrei
> > +				/*
diff mbox series

Patch

diff --git a/hostapd/main.c b/hostapd/main.c
index ce2df81c4a..228ee44b1b 100644
--- a/hostapd/main.c
+++ b/hostapd/main.c
@@ -164,6 +164,59 @@  static int hostapd_driver_init(struct hostapd_iface *iface)
 		return -1;
 	}
 
+#ifdef CONFIG_IEEE80211BE
+		if (conf->mld_ap) {
+			for (i = 0; i < iface->interfaces->count; i++) {
+				struct hostapd_iface *h = iface->interfaces->iface[i];
+				struct hostapd_data *h_hapd = h->bss[0];
+				struct hostapd_bss_config *hconf = h_hapd->conf;
+
+				if (h == iface) {
+					wpa_printf(MSG_ERROR, "Skip own iface");
+					continue;
+				}
+
+				if (!hconf->mld_ap || hconf->mld_id != conf->mld_id) {
+					wpa_printf(MSG_ERROR,
+						   "Skip non matching mld_id");
+					continue;
+				}
+
+				wpa_printf(MSG_DEBUG, "Found matching MLD iface");
+				if (!h_hapd->drv_priv) {
+					wpa_printf(MSG_ERROR,
+						   "Matching MLD BSS not initialized yet");
+					continue;
+				}
+
+				hapd->drv_priv = h_hapd->drv_priv;
+
+				/*
+				 * All interfaces participating in the MLD AP would have
+				 * the same MLD address, which in the interface HW
+				 * address,  while the interface address would be
+				 * derived from the original interface address if BSSID
+				 * is not configured, and otherwise it would be the
+				 * configured BSSID.
+				 */
+				os_memcpy(hapd->mld_addr, h_hapd->mld_addr, ETH_ALEN);
+				if (is_zero_ether_addr(b)) {
+					os_memcpy(hapd->own_addr, h_hapd->mld_addr, ETH_ALEN);
+					random_mac_addr_keep_oui(hapd->own_addr);
+				} else {
+					os_memcpy(hapd->own_addr, b, ETH_ALEN);
+				}
+
+				/*
+				 * mark the interface as a secondary interface, as this
+				 * is needed for the de-initialization flow
+				 */
+				hapd->mld_first_bss = h_hapd;
+				goto setup_mld;
+			}
+		}
+#endif /* CONFIG_IEEE80211BE */
+
 	/* Initialize the driver interface */
 	if (!(b[0] | b[1] | b[2] | b[3] | b[4] | b[5]))
 		b = NULL;
@@ -214,6 +267,20 @@  static int hostapd_driver_init(struct hostapd_iface *iface)
 		return -1;
 	}
 
+#ifdef CONFIG_IEEE80211BE
+		/*
+		 * This is the first interface added to the MLD AP, so have the
+		 * interface HW address be the MLD address and set a link address to
+		 * this interface
+		 */
+		if (hapd->conf->mld_ap) {
+			os_memcpy(hapd->mld_addr, hapd->own_addr, ETH_ALEN);
+			random_mac_addr_keep_oui(hapd->own_addr);
+		}
+
+	setup_mld:
+#endif /* CONFIG_IEEE80211BE */
+
 	if (hapd->driver->get_capa &&
 	    hapd->driver->get_capa(hapd->drv_priv, &capa) == 0) {
 		struct wowlan_triggers *triggs;
@@ -246,6 +313,23 @@  static int hostapd_driver_init(struct hostapd_iface *iface)
 		iface->ema_max_periodicity = capa.ema_max_periodicity;
 	}
 
+#ifdef CONFIG_IEEE80211BE
+	if (hapd->conf->mld_ap) {
+		if (!(iface->drv_flags2 & WPA_DRIVER_FLAGS2_MLO)) {
+			wpa_printf(MSG_DEBUG, "MLD: not supported by driver");
+			return -1;
+		}
+
+		wpa_printf(MSG_DEBUG,
+			   "MLD: Set link_id=%u, mld_addr=" MACSTR ", own_addr=" MACSTR,
+			   hapd->conf->mld_link_id,
+			   MAC2STR(hapd->mld_addr),
+			   MAC2STR(hapd->own_addr));
+
+		hostapd_drv_link_add(hapd, hapd->conf->mld_link_id,
+				     hapd->own_addr);
+	}
+#endif /* CONFIG_IEEE80211BE */
 	return 0;
 }
 
diff --git a/src/ap/ap_drv_ops.h b/src/ap/ap_drv_ops.h
index 93b2244990..be280de218 100644
--- a/src/ap/ap_drv_ops.h
+++ b/src/ap/ap_drv_ops.h
@@ -435,4 +435,16 @@  hostapd_drv_register_frame(struct hostapd_data *hapd, u16 type,
 }
 #endif /* CONFIG_TESTING_OPTIONS */
 
+#ifdef CONFIG_IEEE80211BE
+static inline int hostapd_drv_link_add(struct hostapd_data *hapd,
+				       u8 link_id, const u8 *addr)
+{
+	if (!hapd->driver || !hapd->drv_priv || !hapd->driver->link_add)
+		return -1;
+
+	return hapd->driver->link_add(hapd->drv_priv, link_id, addr);
+
+}
+#endif /* CONFIG_IEEE80211BE */
+
 #endif /* AP_DRV_OPS */
diff --git a/src/ap/hostapd.c b/src/ap/hostapd.c
index 58492e51ed..9abfb5fa5f 100644
--- a/src/ap/hostapd.c
+++ b/src/ap/hostapd.c
@@ -393,6 +393,25 @@  static int hostapd_broadcast_wep_set(struct hostapd_data *hapd)
 #endif /* CONFIG_WEP */
 
 
+static void hostapd_clear_drv_priv(struct hostapd_data *hapd)
+{
+	u8 i;
+
+	for (i = 0; i < hapd->iface->interfaces->count; i++) {
+		struct hostapd_iface *iface = hapd->iface->interfaces->iface[i];
+
+		if (hapd->iface == iface)
+			continue;
+
+		if (iface->bss && iface->bss[0] &&
+		    iface->bss[0]->mld_first_bss == hapd)
+			iface->bss[0]->drv_priv = NULL;
+	}
+
+	hapd->drv_priv = NULL;
+}
+
+
 void hostapd_free_hapd_data(struct hostapd_data *hapd)
 {
 	os_free(hapd->probereq_cb);
@@ -449,7 +468,7 @@  void hostapd_free_hapd_data(struct hostapd_data *hapd)
 			 * driver wrapper may have removed its internal instance
 			 * and hapd->drv_priv is not valid anymore.
 			 */
-			hapd->drv_priv = NULL;
+			hostapd_clear_drv_priv(hapd);
 		}
 	}
 
@@ -2780,8 +2799,9 @@  void hostapd_interface_deinit_free(struct hostapd_iface *iface)
 	wpa_printf(MSG_DEBUG, "%s: driver=%p drv_priv=%p -> hapd_deinit",
 		   __func__, driver, drv_priv);
 	if (driver && driver->hapd_deinit && drv_priv) {
-		driver->hapd_deinit(drv_priv);
-		iface->bss[0]->drv_priv = NULL;
+		if (!iface->bss[0]->mld_first_bss)
+			driver->hapd_deinit(drv_priv);
+		hostapd_clear_drv_priv(iface->bss[0]);
 	}
 	hostapd_interface_free(iface);
 }
@@ -2796,13 +2816,14 @@  static void hostapd_deinit_driver(const struct wpa_driver_ops *driver,
 	wpa_printf(MSG_DEBUG, "%s: driver=%p drv_priv=%p -> hapd_deinit",
 		   __func__, driver, drv_priv);
 	if (driver && driver->hapd_deinit && drv_priv) {
-		driver->hapd_deinit(drv_priv);
+		if (!hapd_iface->bss[0]->mld_first_bss)
+			driver->hapd_deinit(drv_priv);
 		for (j = 0; j < hapd_iface->num_bss; j++) {
 			wpa_printf(MSG_DEBUG, "%s:bss[%d]->drv_priv=%p",
 				   __func__, (int) j,
 				   hapd_iface->bss[j]->drv_priv);
 			if (hapd_iface->bss[j]->drv_priv == drv_priv) {
-				hapd_iface->bss[j]->drv_priv = NULL;
+				hostapd_clear_drv_priv(hapd_iface->bss[j]);
 				hapd_iface->extended_capa = NULL;
 				hapd_iface->extended_capa_mask = NULL;
 				hapd_iface->extended_capa_len = 0;
@@ -3143,8 +3164,14 @@  int hostapd_add_iface(struct hapd_interfaces *interfaces, char *buf)
 		conf_file = ptr + 7;
 
 	for (i = 0; i < interfaces->count; i++) {
+		bool mld_ap = false;
+
+#ifdef CONFIG_IEEE80211BE
+		mld_ap = interfaces->iface[i]->conf->bss[0]->mld_ap;
+#endif /* CONFIG_IEEE80211BE */
+
 		if (!os_strcmp(interfaces->iface[i]->conf->bss[0]->iface,
-			       buf)) {
+			       buf) && !mld_ap) {
 			wpa_printf(MSG_INFO, "Cannot add interface - it "
 				   "already exists");
 			return -1;
diff --git a/src/ap/hostapd.h b/src/ap/hostapd.h
index ed2ff45877..6c0cece36e 100644
--- a/src/ap/hostapd.h
+++ b/src/ap/hostapd.h
@@ -174,6 +174,8 @@  struct hostapd_data {
 	unsigned int reenable_beacon:1;
 
 	u8 own_addr[ETH_ALEN];
+	u8 mld_addr[ETH_ALEN];
+	struct hostapd_data *mld_first_bss;
 
 	int num_sta; /* number of entries in sta_list */
 	struct sta_info *sta_list; /* STA info list head */