netdev - Re: [RFC] tcp: add support for scheduling TCP options on TCP sockets

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 for Android: free password hash cracker in your pocket

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

Date:	Wed, 7 May 2014 17:04:36 +0300
From:	Octavian Purdila <octavian.purdila@...el.com>
To:	Christoph Paasch <christoph.paasch@...ouvain.be>
Cc:	David Miller <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [RFC] tcp: add support for scheduling TCP options on TCP sockets

On Wed, May 7, 2014 at 4:46 PM, Christoph Paasch
<christoph.paasch@...ouvain.be> wrote:
> Hello Octavian,
>
> On 07/05/14 - 07:30:23, Octavian Purdila wrote:
>> On Wed, May 7, 2014 at 8:38 AM, David Miller <davem@...emloft.net> wrote:
>> > Sorry I don't like this.
>> >
>> > Walking a linked list unnecessary is going to add overhead to every
>> > single packet transmission.  I think more people want our TCP stack to
>> > be fast (everyone) than those who want option processing to be
>> > abstracted enough to be modular (you).
>> >
>> > Just make the intrusive changes, they are necessary as they force you
>> > to think fully about how one option might interact with another.
>> >
>>
>> Unfortunately skb_tcp_cb does not have enough space to hold
>> information for new large options. To work around that, the MPTCP
>> implementation is pushing the option data in the skb and then
>> occasionally uses the following when the pskb_copy is used:
>>
>> -               else
>> +               if (unlikely(skb_cloned(skb))) {
>> +                       struct sk_buff *newskb;
>> +                       if (mptcp_is_data_seq(skb))
>> +                               skb_push(skb, MPTCP_SUB_LEN_DSS_ALIGN +
>> +                                             MPTCP_SUB_LEN_ACK_ALIGN +
>> +                                             MPTCP_SUB_LEN_SEQ_ALIGN);
>> +
>> +                       newskb = pskb_copy(skb, gfp_mask);
>> +
>> +                       if (mptcp_is_data_seq(skb)) {
>> +                               skb_pull(skb, MPTCP_SUB_LEN_DSS_ALIGN +
>> +                                             MPTCP_SUB_LEN_ACK_ALIGN +
>> +                                             MPTCP_SUB_LEN_SEQ_ALIGN);
>> +                               if (newskb)
>> +                                       skb_pull(newskb,
>> MPTCP_SUB_LEN_DSS_ALIGN +
>> +
>> MPTCP_SUB_LEN_ACK_ALIGN +
>> +
>> MPTCP_SUB_LEN_SEQ_ALIGN);
>> +                       }
>> +                       skb = newskb;
>> +               } else {
>>                         skb = skb_clone(skb, gfp_mask);
>> +               }
>>
>> MPTCP has many other intrusive changes in the TCP stack. To avoid that
>> complexity, we could do the bulk of the implementation in a separate
>> layer, on top of TCP. But we would need a mechanism to pass the
>> options down to the TCP layer somehow.
>
> Why not extend the head-space of the linear data of the skb as we discussed
> already previously on mptcp-dev? Just in a similar way as 'struct can_skb_priv'
> is being used. This would avoid this expensive list-processing and clean up
> the above example you give.
>
> Or did something else prevented to do it in such a way?
>

You mean storing options at skb->head? Wouldn't we have the same issue
as above for pskb_copy?
--
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