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

[Christoph Paasch <christoph.paasch@uclouvain.be>]

On 07/05/14 - 14:04:36, Octavian Purdila wrote:
> On Wed, May 7, 2014 at 4:46 PM, Christoph Paasch
> <christoph.paasch@uclouvain.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