Re: [PATCH v2 1/2] mac80211: add A-MSDU tx support

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2016-02-24 at 10:28 +0100, Felix Fietkau wrote:
> 
> +	/*
> +	 * HT A-MPDU limits maximum MPDU size to 4095 bytes. Since aggregation
> +	 * sessions are started/stopped without txq flush, use the limit here
> +	 * to avoid having to de-aggregate later.
> +	 */
> +	if (skb->len + head->len > 4095 &&
> +	    !sta->sta.vht_cap.vht_supported)
> +		goto out;

I'm not entirely happy with this. You're silently assuming that when
VHT is supported, HT MCSes will never be used.

This is (I think) true for minstrel_ht, but at the very least you
should also document it along with the max_rc_amsdu_len then, which btw
I was going to reword to:

+ * @IEEE80211_HW_TX_AMSDU: Hardware (or driver) supports software aggregated
+ *     A-MSDU frames. Requires software tx queueing and fast-xmit support.
+ *     When not using minstrel/minstrel_ht rate control, the driver needs to
+ *     limit the maximum A-MSDU size based on the current tx rate by setting
+ *     max_rc_amsdu_len in struct ieee80211_sta to avoid mac80211 building
+ *     A-MSDUs that require too much airtime (are too long for a TXOP.)


All that said, I'm not sure how much value there really is in
aggregating that much? I'd think the value of A-MSDU really is more for
lots of small frames like TCP ACKs, since the PER goes up exponentially
with the BER for A-MSDU (unlike A-MPDU.)

johannes