On 01/16/2013 11:14 PM, Johannes Berg wrote:
On Mon, 2013-01-07 at 16:04 +0100, Marco Porsch wrote:mode determines when non-peer mesh STA may send Probe Requests and Mesh PeeringPlease break lines to less than 72 characters, I personally prefer around 60 or so but I'll apply 72 too.+static inline bool ieee80211_has_qos_mesh_ps(__le16 qc) +{ + return (qc & cpu_to_le16(IEEE80211_QOS_CTL_MESH_PS_LEVEL)) != 0;bool means you don't need the !=0 and parentheses.
Actually, then it pops the following warning: CHECK net/mac80211/mesh_ps.cinclude/linux/ieee80211.h:584:19: warning: incorrect type in return expression (different base types)
include/linux/ieee80211.h:584:19: expected bool
include/linux/ieee80211.h:584:19: got restricted __le16
Other functions in that file have following style:
static inline int ieee80211_has_pm(__le16 fc)
{
return (fc & cpu_to_le16(IEEE80211_FCTL_PM)) != 0;
}
What do you recommend?
@@ -1208,6 +1214,7 @@ struct ieee802_11_elems { struct ieee80211_meshconf_ie *mesh_config; u8 *mesh_id; u8 *peering; + u8 *awake_window;maybe that should be an __le16 pointer?+ /* we need some delay here, otherwise the announcement + * Null would arrive before the CONFIRM + */ + ieee80211_mps_set_sta_local_pm(sta, mshcfg->power_mode, + 100);??? How can a *delay* ever cause correctness? That doesn't seem right.+ /* announce peer-specific power mode transition + * see IEEE802.11-2012 13.14.3.2 and 13.14.3.3 + */I feel a bit bad about asking this, but mac80211 actually doesn't use the "networking" style, all other comments have /* * announce ... * ... */ so please adjust yours as well. I think I deleted this, but nonetheless:static void mpsp_trigger_send(struct sta_info *sta, bool rspi, bool eosp)that easily fits on one line.+/** + * mps_frame_deliver - transmit frames during mesh powersave + * + * @sta: STA info to transmit to + * @n_frames: number of frames to transmit. -1 for all + */ +static void mps_frame_deliver(struct sta_info *sta, int n_frames) +{ + struct ieee80211_sub_if_data *sdata = sta->sdata; + struct ieee80211_local *local = sdata->local; + int ac; + struct sk_buff_head frames; + struct sk_buff *skb; + bool more_data = false; + + skb_queue_head_init(&frames);You really don't need a spinlock for on-stack queues, so you should use the __ versions of the functions modifying "frames".+ if (!skb) + break; + n_frames--;I guess you're assuming there never will be > 2^31 frames ;-)+ skb_queue_tail(&frames, skb);__skb_queue_tail(), etc.+ } + + if (!skb_queue_empty(&sta->tx_filtered[ac]) || + !skb_queue_empty(&sta->ps_tx_buf[ac])) { + more_data = true; + }No need for the braces+ /* in a MPSP make sure the last skb is a QoS Data frame */ + if (test_sta_flag(sta, WLAN_STA_MPSP_OWNER)) + mpsp_qos_null_append(sta, &frames);+ mps_dbg(sta->sdata, "sending %d frames to PS STA %pM\n", + skb_queue_len(&frames), sta->sta.addr); + + /* prepare collected frames for transmission */ + skb_queue_walk(&frames, skb) { + struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); + struct ieee80211_hdr *hdr = (void *) skb->data; + + /* Tell TX path to send this frame even though the + * STA may still remain is PS mode after this frame + * exchange. + */ + info->flags |= IEEE80211_TX_CTL_NO_PS_BUFFER;I'm not convinced that the PS integration with drivers here is correct at all, but I don't really care all that much either.+ * @WLAN_STA_MPSP_OWNER: local STA is owner of a mesh Peer Service Period. + * @WLAN_STA_MPSP_RECIPIENT: local STA is recipient of a MPSP. + * @WLAN_STA_MPS_WAIT_FOR_BEACON: STA beacon is imminent -> stay awake + * @WLAN_STA_MPS_WAIT_FOR_CAB: STA multicast frames are imminent -> stay awake?? There's also a compiler warning -- variable shadowing -- that this probably solves: @@ -487,8 +487,6 @@ static void mps_frame_deliver(struct sta_info *sta, int n_frames) /* collect frame(s) from buffers */ for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) { - struct sk_buff *skb; - while (n_frames != 0) { skb = skb_dequeue(&sta->tx_filtered[ac]); if (!skb) { 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
