On 10/22/2015 05:27 AM, Eric Dumazet wrote:
> On Thu, 2015-10-22 at 00:14 +0000, Grumbach, Emmanuel wrote:
>
>>
>> Well. I guess I should at least check, but even with very small MSS, our
>> device supports up to 20 pointers for the same 802.11 packet: 2 are for
>> metadata. So basically, so leaves me only 18 pointers. for each MSS I
>> need at least 2 (one for the headers and one for the payload), so I will
>> have at most 9 of these for one packet, even with a tiny MSS.
>>
>
> I did not see in your patch where you made the checks about 18 segs in a
> TSO packet ?
It is in the other patch: iwlwifi: mvm: send large SKBs to the transport
mvm is the op_mode and the op_mode needs to make sure that the payload
fits in one 802.11 packet AND it doesn't exhaust the number of pointers.
I'll add a comment here.
>
>> I agree that all this should be added to the code in a comment.
>> Speaking of which...
>> int tso_count_descs(struct sk_buff *skb)
>> {
>> /* The Marvell Way */
>> return skb_shinfo(skb)->gso_segs * 2 + skb_shinfo(skb)->nr_frags;
>> }
>>
>> What if there is some payload in the header?
>> To me it sounds safer to return:
>>
>> skb_shinfo(skb)->gso_segs * 2 + skb_shinfo(skb)->nr_frags + 1;
>>
>> or maybe to test if there is some payload in the header and then add 1?
>> If there is payload in the header, it should be considered as another
>> frag, shouldn't it?
>
> Minimal count is gso_segs (one per MSS)
>
> Then you have to add extra for the cases we have a mss spanning a frag
> in skb.
>
> Thats a max of (skb_shinfo(skb)->nr_frags - 1) + (data_in_head() ? 1 :
> 0);
> So I believe formula would be correct.
>
I needed a piece of paper and a few drawings to understand you are
right... :)
--
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
