netdev - Re: [RFC v3 1/2] iwlwifi: pcie: allow to build an A-MSDU using TSO core

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 22 Oct 2015 06:07:42 +0000
From:	"Grumbach, Emmanuel" <emmanuel.grumbach@...el.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
CC:	"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"Sharon, Sara" <sara.sharon@...el.com>,
	"ido@...ery.com" <ido@...ery.com>
Subject: Re: [RFC v3 1/2] iwlwifi: pcie: allow to build an A-MSDU using TSO
 core



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 netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html