On Thu, 2008-02-21 at 20:06 -0800, Luis Carlos Cobo wrote:
> Added support for mesh id and mesh path operation.
Looks great, a few minor comments.
> +/**
> + * struct vif_params - describes virtual interface parameters
> + * @mesh_id: mesh ID to use
> + * @mesh_id_len: length of the mesh ID
> + */
> +struct vif_params {
> + u8 *mesh_id;
> + int mesh_id_len;
> +};
> int (*add_virtual_intf)(struct wiphy *wiphy, char *name,
> - enum nl80211_iftype type, u32 *flags);
> + enum nl80211_iftype type, u32 *flags,
> + struct vif_params *params);
I think we should move flags into the params struct, but we can do that
in a follow-up patch.
> + int (*dump_station)(struct wiphy *wiphy, struct net_device *dev,
> + int idx, u8 *mac, struct station_info *sinfo);
How do you use this? I can see that the "idx" is used to iterate, but
this cannot possibly provide race-free access because the implementation
in mac80211's cfg.c needs to give up the lock. I think we need to change
this to
int (*dump_stations)(wiphy, dev,
void (*callback)(wiphy, dev, data, mac, sinfo),
void *data);
The same seems to apply to nl80211_dump_mpath() and the corresponding
callback.
An alternative would be to provide start_station_dump() and
stop_station_dump() callbacks for the locking, or, something I wouldn't
really like to see, document the interface to lock when idx == 0 and
unlock when the return value is -ENOENT or something...
The rest looks fine.
johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
