diff mbox series

AP: Add configuration option to specify the desired MLD address

Message ID 20230618141504.155026-1-andrei.otcheretianski@intel.com
State Changes Requested
Headers show
Series AP: Add configuration option to specify the desired MLD address | expand

Commit Message

Andrei Otcheretianski June 18, 2023, 2:15 p.m. UTC 
From: Ilan Peer <ilan.peer@intel.com>

Add mld_addr configuration option to set MLD address.
The already existing bssid configuration option can be used to
control the MLD link addresses.

Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
---
 hostapd/config_file.c |  6 ++++++
 hostapd/main.c        | 12 +++++++++++-
 src/ap/ap_config.h    |  3 +++
 3 files changed, 20 insertions(+), 1 deletion(-)

Comments

Jouni Malinen July 19, 2023, 4:51 p.m. UTC | #1 
On Sun, Jun 18, 2023 at 05:15:04PM +0300, Andrei Otcheretianski wrote:
> Add mld_addr configuration option to set MLD address.
> The already existing bssid configuration option can be used to
> control the MLD link addresses.

> diff --git a/hostapd/config_file.c b/hostapd/config_file.c
> @@ -4776,6 +4776,12 @@ static int hostapd_config_fill(struct hostapd_config *conf,
>  		bss->mld_ap = !!atoi(pos);
>  	} else if (os_strcmp(buf, "mld_id") == 0) {
>  		bss->mld_id = atoi(pos);
> +	} else if (os_strcmp(buf, "mld_addr") == 0) {
> +		if (hwaddr_aton(pos, bss->mld_addr)) {
> +			wpa_printf(MSG_ERROR,
> +				   "Line %d: invalid mld_addr", line);
> +			return 1;
> +		}
>  #endif /* CONFIG_IEEE80211BE */

It would be good to document new hostapd configuration parameters in
hostapd/hostapd.conf.

> diff --git a/hostapd/main.c b/hostapd/main.c
>  	/* Initialize the driver interface */
> -	if (!(b[0] | b[1] | b[2] | b[3] | b[4] | b[5]))
> +	if (is_zero_ether_addr(b))
>  		b = NULL;

How is this related to the other changes in the patch or the commit
message? This feels like something completely independent that should be
in its own commit.

> @@ -241,7 +241,13 @@ static int hostapd_driver_init(struct hostapd_iface *iface)
> +#ifndef CONFIG_IEEE80211BE
>  	params.bssid = b;
> +#else
> +	if (!is_zero_ether_addr(hapd->conf->mld_addr) &&
> +	    hapd->conf->mld_ap)
> +		params.bssid = hapd->conf->mld_addr;
> +#endif /* !CONFIG_IEEE80211BE */

Is this really correct for builds that define CONFIG_IEEE80211BE but do
not enable MLO? Why would params.bssid not be set in those cases?
Wouldn't this break capability of configuring the BSSID for non-MLO AP?
 
> @@ -278,6 +284,10 @@ static int hostapd_driver_init(struct hostapd_iface *iface)
>  		random_mac_addr_keep_oui(hapd->own_addr);
>  		hapd->mld_next_link_id = 0;
>  		hapd->mld_link_id = hapd->mld_next_link_id++;
> +		if (!b)
> +			random_mac_addr_keep_oui(hapd->own_addr);
> +		else
> +			os_memcpy(hapd->own_addr, b, ETH_ALEN);

What is this trying to do and how is this related to the other changes?
random_mac_addr_keep_oui() would be called twice if b == NULL and that
does not sound correct. Why would hostapd_driver_init() modify
hapd->own_addr?
diff mbox series

Patch

diff --git a/hostapd/config_file.c b/hostapd/config_file.c
index 412ca8a9f8..d76a1a4620 100644
--- a/hostapd/config_file.c
+++ b/hostapd/config_file.c
@@ -4776,6 +4776,12 @@  static int hostapd_config_fill(struct hostapd_config *conf,
 		bss->mld_ap = !!atoi(pos);
 	} else if (os_strcmp(buf, "mld_id") == 0) {
 		bss->mld_id = atoi(pos);
+	} else if (os_strcmp(buf, "mld_addr") == 0) {
+		if (hwaddr_aton(pos, bss->mld_addr)) {
+			wpa_printf(MSG_ERROR,
+				   "Line %d: invalid mld_addr", line);
+			return 1;
+		}
 #endif /* CONFIG_IEEE80211BE */
 	} else {
 		wpa_printf(MSG_ERROR,
diff --git a/hostapd/main.c b/hostapd/main.c
index aebdffd1fd..b3cce4159e 100644
--- a/hostapd/main.c
+++ b/hostapd/main.c
@@ -218,7 +218,7 @@  static int hostapd_driver_init(struct hostapd_iface *iface)
 #endif /* CONFIG_IEEE80211BE */
 
 	/* Initialize the driver interface */
-	if (!(b[0] | b[1] | b[2] | b[3] | b[4] | b[5]))
+	if (is_zero_ether_addr(b))
 		b = NULL;
 
 	os_memset(&params, 0, sizeof(params));
@@ -241,7 +241,13 @@  static int hostapd_driver_init(struct hostapd_iface *iface)
 		params.global_priv = global.drv_priv[i];
 		break;
 	}
+#ifndef CONFIG_IEEE80211BE
 	params.bssid = b;
+#else
+	if (!is_zero_ether_addr(hapd->conf->mld_addr) &&
+	    hapd->conf->mld_ap)
+		params.bssid = hapd->conf->mld_addr;
+#endif /* !CONFIG_IEEE80211BE */
 	params.ifname = hapd->conf->iface;
 	params.driver_params = hapd->iconf->driver_params;
 	params.use_pae_group_addr = hapd->conf->use_pae_group_addr;
@@ -278,6 +284,10 @@  static int hostapd_driver_init(struct hostapd_iface *iface)
 		random_mac_addr_keep_oui(hapd->own_addr);
 		hapd->mld_next_link_id = 0;
 		hapd->mld_link_id = hapd->mld_next_link_id++;
+		if (!b)
+			random_mac_addr_keep_oui(hapd->own_addr);
+		else
+			os_memcpy(hapd->own_addr, b, ETH_ALEN);
 	}
 
 setup_mld:
diff --git a/src/ap/ap_config.h b/src/ap/ap_config.h
index 777aa5af05..83fdbaad10 100644
--- a/src/ap/ap_config.h
+++ b/src/ap/ap_config.h
@@ -944,6 +944,9 @@  struct hostapd_bss_config {
 
 	/* The MLD ID to which the AP MLD is affiliated with */
 	u8 mld_id;
+
+	/* The AP's MLD address within the MLD AP */
+	u8 mld_addr[ETH_ALEN];
 #endif /* CONFIG_IEEE80211BE */
 };