Re: [PATCH 04/13] mbssid: get and set configuration parameters

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 02, 2022 at 02:26:25PM -0800, Aloka Dixit wrote:
> diff --git a/src/ap/beacon.c b/src/ap/beacon.c

> +static void hostapd_set_mbssid_beacon(struct hostapd_data *hapd,
> +				      struct wpa_driver_ap_params *params)
> +{
> +	struct hostapd_iface *iface = hapd->iface;
> +
> +	if (!iface->conf->mbssid || iface->num_bss == 1)
> +		return;
> +
> +	if (!iface->mbssid_max_interfaces) {
> +		wpa_printf(MSG_DEBUG,
> +			   "MBSSID: Driver doesn't support multi-BSSID advertisements");
> +		return;
> +	} else if (iface->num_bss > iface->mbssid_max_interfaces) {
> +		wpa_printf(MSG_DEBUG,
> +			   "MBSSID: Driver supports maximum %u interfaces for multi-BSSID advertisements",
> +			   iface->mbssid_max_interfaces);
> +		return;
> +	}

Can these be reached in practice? Shouldn't such cases be prevented from
starting AP operation?

> +	params->mbssid_tx_iface = hostapd_mbssid_get_tx_bss(hapd)->conf->iface;
> +	params->mbssid_index = hostapd_mbssid_get_bss_index(hapd);
> +	params->mbssid_count = iface->num_bss;
> +	return;
> +}

No "return;" at the end of void function.

> @@ -1149,7 +1175,6 @@ static u8 * hostapd_probe_resp_offloads(struct hostapd_data *hapd,
>  	/* Generate a Probe Response template for the non-P2P case */
>  	return hostapd_gen_probe_resp(hapd, NULL, 0, resp_len);
>  }
> -
>  #endif /* NEED_AP_MLME */
>  
>  

Please do not include unrelated (and in this case, an undesired
whitespace change in general) changes in the patch.

> diff --git a/src/drivers/driver.h b/src/drivers/driver.h
> @@ -1582,6 +1582,21 @@ struct wpa_driver_ap_params {
> +	/**
> +	 * mbssid_tx_iface - Transmitting interface of the set
> +	 */
> +	const char *mbssid_tx_iface;
> +
> +	/**
> +	 * mbssid_index - The index of this BSS in the group
> +	 */
> +	unsigned int mbssid_index;
> +
> +	/**
> +	 * mbssid_count - Total number of BSSs in the group
> +	 */
> +	unsigned int mbssid_count;
>  };

Why is that mbssid_count added here? It does not seem to be used in any
of the following patches.. Or well, it is just used to check whether
MBSSID mechanism is to be enabled and a single bool would seem to be a
clearer way of indicating that to cover the case of mbssid_index == 0.
Or well, I guess even that bool would be unnecessary since
mbssid_tx_iface != NULL would indicate if this is enabled for both the
transmitting BSSID and the nontransmitting ones.
 
-- 
Jouni Malinen                                            PGP id EFC895FA

_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap



[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux