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-next>] [day] [month] [year] [list]

Date:	Wed, 7 May 2014 16:11:08 +0200
From:	Christoph Paasch <christoph.paasch@...ouvain.be>
To:	Octavian Purdila <octavian.purdila@...el.com>
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 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.


Cheers,
Christoph

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