Re: [PATCH RFC] mac80211_hwsim

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 3778 bytes --]


> Here's a completely untested (well, it compiles) patch against
> wireless-test. I'll try to do some testing tomorrow, but if someone is
> interested in taking a closer look at a version that is converted to use
> the new data structures for channel/rate configuration, here's a
> preliminary version. All comments are welcome.
> 
> As far as functionality is concerned, this is supposed to match with the
> 2.6.25-based version I have in hostap.git, i.e., the only changes are to
> convert the newer version of mac80211 API. Since I'm frequently using
> the latest 2.6.x release kernel instead of wireless-test (and would
> assume I'm not the only one doing that), I'm thinking of maintaining two
> versions of the module. One would be in wireless-test and maintained
> just like any other in-tree driver. The other one is in my hostap.git
> repository and is maintained as an out-of-tree version that is based on
> the latest (or well, relatively close to the latest, i.e., whatever I
> happen to be using ;-) released kernel. Of course, once the in-tree
> version hits a released kernel, out-of-tree version can die.


Sounds good to me.

> PS.
> 
> The URL in Kconfig is just a placeholder and I'll add the information in
> README and hostapd/wpa_supplicant example configuration files there
> before this gets submitted properly.

Should we add it to Documentation/networking/ instead? Don't care much,
but it might be easier to track version changes that way.

> +static const struct ieee80211_rate hwsim_rates[] = {
> +	{ .bitrate = 10 },
> +	{ .bitrate = 20, .flags = IEEE80211_RATE_SHORT_PREAMBLE },
> +	{ .bitrate = 55, .flags = IEEE80211_RATE_SHORT_PREAMBLE },
> +	{ .bitrate = 110, .flags = IEEE80211_RATE_SHORT_PREAMBLE },
> +	{ .bitrate = 60, .flags = IEEE80211_RATE_ERP_G },

You don't need to add the ERP flag manually, it'll be added based on the
rate automatically.

> +	if (is_multicast_ether_addr(hdr->addr1))
> +		ack = 1;

That seems a bit weird to me. mac80211 shouldn't request an ACK for
mcast, but why fake one?

> ... beacon ...
> +	if (vif->type != IEEE80211_IF_TYPE_AP)
> +		return;

That could support mesh as well, in mesh-beacon-like-AP mode rather than
mesh-beacon-like-IBSS.

> +                   data->channel->band != data2->channel->band ||
> +                   data->channel->center_freq != data2->channel->center_freq)

The band check is useless since the frequency is in MHz, it's mostly to
know which band table to look up things in if necessary.

> +static void mac80211_hwsim_beacon(unsigned long arg)
> +{
> +	struct ieee80211_hw *hw = (struct ieee80211_hw *) arg;
> +	struct mac80211_hwsim_data *data = hw->priv;
> +
> +	if (!data->started || !data->radio_enabled)
> +		return;
> +
> +	ieee80211_iterate_active_interfaces(hw, mac80211_hwsim_beacon_tx, hw);

Cool, another user of this API :)

> +	data->rx_filter = 0;
> +	if (*total_flags & FIF_PROMISC_IN_BSS)
> +		data->rx_filter |= FIF_PROMISC_IN_BSS;
> +	if (*total_flags & FIF_ALLMULTI)
> +		data->rx_filter |= FIF_ALLMULTI;
> +
> +	*total_flags = data->rx_filter;

You don't actually seem to use these flags though. Should be added at
some point, I think.

> +		memcpy(data->channels, hwsim_channels, sizeof(hwsim_channels));
> +		memcpy(data->rates, hwsim_rates, sizeof(hwsim_rates));
> +		data->band.channels = data->channels;
> +		data->band.n_channels = ARRAY_SIZE(hwsim_channels);
> +		data->band.bitrates = data->rates;
> +		data->band.n_bitrates = ARRAY_SIZE(hwsim_rates);

You shouldn't need to memcpy these, just point to the static
allocations.

> +		hw->wiphy->bands[IEEE80211_BAND_2GHZ] = &data->band;

You can even allocate the band struct statically.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]