Some comments... In these cases > + init_timer(&ifsta->admit_timer); > + ifsta->admit_timer.data = (unsigned long) dev; > + ifsta->admit_timer.function = ieee80211_admit_refresh;
> +void ieee80211_send_addts(struct net_device *dev,
> + struct ieee802_11_elem_tspec *tspec)
> +void wmm_send_addts(struct net_device *dev, struct ieee802_11_elem_tspec
> *tspec)
> +void ieee80211_send_delts(struct net_device *dev, u8 tsid, u8 direction,
> + u32 medium_time)
> +void wmm_send_delts(struct net_device *dev, u8 tsid, u8 direction,
> + u32 medium_time)
please don't use the device as the argument but rather the sdata. Using
the device only to unwrap it to the sdata again isn't really nice.
> + mgmt = (struct ieee80211_mgmt *) skb_put(skb, 24);
> + memset(mgmt, 0, 24);
> + memcpy(mgmt->da, ifsta->bssid, ETH_ALEN);
> + memcpy(mgmt->sa, dev->dev_addr, ETH_ALEN);
> + memcpy(mgmt->bssid, ifsta->bssid, ETH_ALEN);
No need to zero out the structure if you set all fields anyway.
> + mgmt = (struct ieee80211_mgmt *) skb_put(skb, 24);
> + memset(mgmt, 0, 24);
> + memcpy(mgmt->da, ifsta->bssid, ETH_ALEN);
> + memcpy(mgmt->sa, dev->dev_addr, ETH_ALEN);
> + memcpy(mgmt->bssid, ifsta->bssid, ETH_ALEN);
> + mgmt->frame_control = IEEE80211_FC(IEEE80211_FTYPE_MGMT,
> + IEEE80211_STYPE_ACTION);
same here.
> + mgmt = (struct ieee80211_mgmt *) skb_put(skb, 24);
> + memset(mgmt, 0, 24);
and here, and in a few more places.
> +static u32 calculate_mpdu_exchange_time(struct ieee802_11_elem_tspec *tspec)
> +{
> + /*
> + * MPDUExchangeTime = duration(Nominal MSDU Size, Min PHY Rate) +
> + * SIFS + ACK duration
> + */
> + return 5000;
> +}
Is this correct? Aren't some of those things variable?
> + printk(KERN_DEBUG "Dialog_token: %d, TID: %u, Direction: %u, PSB: %d, "
> + "UP: %d\n", mgmt->u.action.u.wme_action.dialog_token,
> + tspec->ts_info.tsid, tspec->ts_info.direction,
> + tspec->ts_info.apsd, tspec->ts_info.up);
Can we have those printks optional?
> +static void ieee80211_rx_mgmt_action(struct net_device *dev,
> + if (len < 24 + 1) {
> + printk(KERN_DEBUG "%s: too short (%zd) action frame "
> + "received from " MAC_FMT " - ignored\n",
> + dev->name, len, MAC_ARG(mgmt->sa));
> + return;
> + }
Do we really want this if in all possibilities where we use the data it
needs to be 24+4 long?
> + case WLAN_CATEGORY_DLS:
> + case WLAN_CATEGORY_BACK:
> + default:
> + printk(KERN_ERR "%s: unsupported action category %d\n",
> + dev->name, mgmt->u.action.category);
> + break;
I don't think these are KERN_ERR.
johannes
signature.asc
Description: This is a digitally signed message part
