Re: [PATCH v2] mac80211: do not aggregate frames if max_frags is set to one

[Lorenzo Bianconi <lorenzo.bianconi@redhat.com>]

> First of all, I applied your patch with now, but changed the commit
> message. I hope it still makes sense.

Thx, definitely better than mine :)

> 
> > Ops, maybe I got your point:
> > ieee80211_amsdu_realloc_pad() in ieee80211_amsdu_prepare_head() can
> > expand the headroom on the first frame
> 
> Right.
> 
> > but if ieee80211_amsdu_realloc_pad() on the second one fails, we do
> > not take into account the extra len added on the
> > first subframe. Is that what you mean?
> 
> Yes, that's what I was thinking of, but you described it much better
> than me :)
> 
> If this needs to be addressed, please send a separate patch.

Reviewing the code I guess it is not necessary since pskb_expand_head routine
does not modify head->len (or skb->len). Packet len (if we consider padding)
is only modified in:

memset(skb_push(skb, pad), 0, pad);

and if we hit that point, we will account new skb->len in flow backlog. Do you
agree?

Looking at the code maybe I spotted another issue, I guess there is an
off-by-one issue in 'n' estimation since it does not take into account
the first frame. We hit the line:

while (*frag_tail) {
}

starting from the second subframe, but if the head does not have packet in the
fraglist we will end up having n = 1, while it is actually the second frame.
Does n count just subsequent frames or also the first one?

Regards,
Lorenzo

> 
> johannes