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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Wed, 7 May 2014 18:17:08 +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 5:11 PM, Christoph Paasch
<christoph.paasch@...ouvain.be> wrote:
> On 07/05/14 - 14:04:36, Octavian Purdila wrote:
>> On Wed, May 7, 2014 at 4:46 PM, Christoph Paasch
>> <christoph.paasch@...ouvain.be> wrote:
>> > On 07/05/14 - 07:30:23, Octavian Purdila wrote:
>> >> 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?
>
> Yes, but it could be done in a more "clean" way so that future extensions to
> TCP are no more limited by the limitation of struct tcp_skb_cb.
>
> Basically, allow some memory inside the linear part to be used by the layer
> the skb is currently at and let pskb_copy handle it properly (not like the
> current 'hack' in tcp_transmit_skb).
> This allows extensions at any layer who are not widely enough used to justify increasing
> skb->cb.
>

That would require adding a new field to sk_buff to keep track of how
much we need to copy in pskb_copy. Fortunately it seems it has some
holes we could use.

David, does that seem reasonable?
--
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