On Wed, 2011-07-13 at 16:45 -0700, Thomas Pedersen wrote:
> + if (ieee80211_vif_is_mesh(&sdata->vif))
> + rcu_assign_pointer(sdata->u.mesh.beacon, new);
I don't think this compiles if MESH isn't configured into mac80211. Not
sure adding lots of ifdefs is good, but it's probably the only choice
right now.
> @@ -461,6 +462,9 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
> {
> struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
> struct ieee80211_local *local = sdata->local;
> + struct ieee80211_mgmt *bcn_head;
> + struct beacon_parameters bcn_params;
> + u8 *pos;
>
> local->fif_other_bss++;
> /* mesh ifaces must set allmulti to forward mcast traffic */
> @@ -473,7 +477,44 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
> set_bit(MESH_WORK_HOUSEKEEPING, &ifmsh->wrkq_flags);
> ieee80211_mesh_root_setup(ifmsh);
> ieee80211_queue_work(&local->hw, &sdata->work);
> +
> + /* Build fixed part of mesh beacon. */
> + memset(&bcn_params, 0, sizeof(struct beacon_parameters));
> +
> + /* header + fixed fields + null ssid */
> + bcn_params.head_len = 24 + sizeof(bcn_head->u.beacon) + 2;
> + pos = kmalloc(bcn_params.head_len, GFP_KERNEL | __GFP_ZERO);
Err, kzalloc, don't use __GFP_ZERO.
> + if (mac80211_config_ops.add_beacon(sdata->wdev.wiphy, sdata->dev,
> + &bcn_params))
> + printk(KERN_ERR "Unable to configure mesh beacon\n");
That's horrible. Don't do that. Just do whatever is necessary inline. If
you restructure, you only need like 10% of the code from
ieee80211_config_beacon() anyway.
> @@ -498,6 +540,8 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
> * it no longer is.
> */
> cancel_work_sync(&sdata->work);
> + if (sdata->u.mesh.beacon)
> + mac80211_config_ops.del_beacon(sdata->wdev.wiphy, sdata->dev);
Ditto.
> } else if (ieee80211_vif_is_mesh(&sdata->vif)) {
> - struct ieee80211_mgmt *mgmt;
> - u8 *pos;
> -
> + struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
> #ifdef CONFIG_MAC80211_MESH
> if (!sdata->u.mesh.mesh_id_len)
> goto out;
> #endif
> + beacon = rcu_dereference(ifmsh->beacon);
not going to compile w/o CONFIG_MAC80211_MESH
> + if (beacon) {
> + /* headroom, head length, tail length, mesh_ies and
> + * maximum TIM length */
> + skb = dev_alloc_skb(local->tx_headroom +
> + beacon->head_len +
> + beacon->tail_len + 400);
what's + 400? You can't really think the TIM IE is that long?
> - /* headroom, head length, tail length and maximum TIM length */
> - skb = dev_alloc_skb(local->tx_headroom + 400 +
> - sdata->u.mesh.ie_len);
I guess it was even there before though ... huh?
Couldn't more code here be shared between mesh and AP? I don't see any
difference here between mesh and AP.
Also ... since you'll need PS support eventually anyway, maybe you
should refactor "struct ieee80211_if_ap" into something AP-specific
(currently only VLANs) and the rest, and then use "the rest" in mesh as
well to make all this code more unified.
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
