netdev - Re: [RFC v2 3/3] iwlwifi: mvm: transfer the truesize to the last TSO segment

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, 20 Aug 2015 06:21:44 +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>,
	"ido@...ery.com" <ido@...ery.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"Sharon, Sara" <sara.sharon@...el.com>
Subject: Re: [RFC v2 3/3] iwlwifi: mvm: transfer the truesize to the last
 TSO segment



On 08/19/2015 11:39 PM, Eric Dumazet wrote:
> On Wed, 2015-08-19 at 19:17 +0000, Grumbach, Emmanuel wrote:
> 
>> Hm.. how would net/core/tso.c avoid this?
> 
> Because a driver using these helpers keep around the original LSO packet
> and frees it normally at TX completion time.
> 

Which is why I can't really use it. The complexity is that I have to
(ieee802.11 specification) split an LSO is several 802.11 packets. The
maximal 802.11 packet I can send under ideal condition is 11K long or
so. So I *must* generate several 802.11 frames from one single LSO
packet. OTOH, I can have more than MSS bytes in a 802.11 A-MSDU.

Maybe what would help would be to be able to dynamically change the
maximal size of an LSO packet. That would allow the wifi driver to
ensure that the LSO can fit in a single 802.11 packet. Note that since
the maximal length of the A-MSDU can vary based on link conditions
(since there is only one CRC for the whole A-MSDU, you don't want long
A-MSDUs in bad link conditions) the driver would need to be able to tell
the TCP stack to modify the length of an LSO packet.
To me, this sounds to be ... an overkill?

I'll sum up all this considerations in the v3 I'll send later today.

>> I can't see anything related to truesize there.
>> Note that this work since it is guaranteed that we release the skbs in
>> order.
>>
>>>
>>> (BTW TCP packets do not have sock_wfree as destructor but tcp_wfree(),
>>> yet we want backpressure mostly for TCP stack (TCP Small Queues))
>>>
>>>
>>
>> I am not sure I follow here.
>> You want me to test:
>> if (skb_gso->destructor == tcp_wfree) ?
> 
> 
> Yes.
> 
> Look for example at tcp_gso_segment() (called from skb_gso_segment())
> 
> copy_destructor = gso_skb->destructor == tcp_wfree;
> ...
>         /* Following permits TCP Small Queues to work well with GSO :
>          * The callback to TCP stack will be called at the time last frag
>          * is freed at TX completion, and not right now when gso_skb
>          * is freed by GSO engine
>          */
>         if (copy_destructor) {
>                 swap(gso_skb->sk, skb->sk);
>                 swap(gso_skb->destructor, skb->destructor);
>                 sum_truesize += skb->truesize;
>                 atomic_add(sum_truesize - gso_skb->truesize,
>                            &skb->sk->sk_wmem_alloc);
>         }
> 
> 
>>
>> I checked that code using iperf and saw that I don't get into this if,
>> but I (probably wrongly) assumed that other applications would set a
>> flag on the socket (forgive my ignorance) that would make this if be taken.
> 
> If you do not see skb->destructor == tcp_wfree, then something is
> definitely wrong on your setup.
> 

I'll check this today. Thanks.

> 
> 
> 
--
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