On 13/12/18 - 11:39:18, Florian Westphal wrote:
> Eric Dumazet <eric.duma...@gmail.com> wrote:
> > > If its going to be used as I expect, then the extension could be
> > > discarded after the DSS mapping has been written to the tcp option
> > > space, i.e. before cloning occurs.
> >
> > I do not see how this would work, without also discarding on the master skb
> > the needed info.
>
> Ok, so lets assume this would result in one atomic_inc/dec due to clone
> for now for skbs coming from mptcp socket.
>
> But I don't see why this would have to be.
>
> > > For TCP, thats true. But there are other places that could clone, e.g.
> > > when bridge has to flood-forward.
> > >
> >
> > So you propose a mechanism that forces a preserve on clone, base on
> > existing needs
> > for bridging.
>
> secpath does the same thing:
>
> static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
> {
> ...
> #ifdef CONFIG_XFRM
> new->sp = secpath_get(old->sp);
> #endif
> ...
>
> So I am not proposing anything new.
>
> > > At least in bridge case the 'preseve on clone' is needed, else required
> > > information is missing from the cloned skb.
> > >
> >
> > We need something where MPTCP info does not need to be propagated all the
> > way to the NIC...
>
> Thats whats done in the MPTCP out-of-tree implementation, but I don't
> think its needed.
Yes, it indeed does not need to go all the way down to the NIC.
The info basically "just" needs to be propagated from the MPTCP-layer down
to the TCP-option space. Thus, it needs to remain on the skbs that are
sitting in the TCP-subflow's send-queue and rexmit tree as we need it when
retransmitting.
In tcp_transmit_skb, the clone is done at the beginning. Thus, we could for
example not inc the refcount on the clone and simply pass a pointer to the
original skb to tcp_established_options.
That way it the DSS option stays within the MPTCP/TCP layer and does not
make it down to the NIC.
Christoph
>
> It could just delete the extension before ->queue_xmit() AFAIU.
>
> > This skb extension is an incentive for adding more sticky things in the skbs
> > to violate layering of networking stacks :/
>
> 8-(
>
> Where do you see "layering violations"?
