Re: [MPTCP] [PATCH 1/2] skbuff: Add shared control buffer

Hi everyone, On Thu, 9 Nov 2017, cpaasch(a)apple.com wrote: > On 09/11/17 - 04:32:54, Boris Pismenny wrote: > > +Ilya and Liran > > > > Hi, > > > > > -----Original Message----- > > > From: cpaasch(a)apple.com [mailto:cpaasch@apple.com] > > > Sent: Thursday, November 09, 2017 13:13 > > > To: Mat Martineau <mathew.j.martineau(a)linux.intel.com&gt;; Boris Pismenny > > > <borisp(a)mellanox.com&gt; > > > Cc: mptcp(a)lists.01.org > > > Subject: Re: [MPTCP] [PATCH 1/2] skbuff: Add shared control buffer > > > > > > +Boris > > > > > > On 20/10/17 - 16:02:31, Mat Martineau wrote: > > > > The sk_buff control buffer is of limited size, and cannot be enlarged > > > > without significant impact on systemwide memory use. However, additional > > > > per-packet state is needed for some protocols, like Multipath TCP. > > > > > > > > An optional shared control buffer placed after the normal struct > > > > skb_shared_info can accomodate the necessary state without imposing > > > > extra memory usage or code changes on normal struct sk_buff > > > > users. __alloc_skb will now place a skb_shared_info_ext structure at > > > > skb->end when given the SKB_ALLOC_SHINFO_EXT flag. Most users of struct > > > > sk_buff continue to use the skb_shinfo() macro to access shared > > > > info. skb_shinfo(skb)->is_ext is set if the extended structure is > > > > available, and cleared if it is not. > > > > > > > > pskb_expand_head will preserve the shared control buffer if it is present. > > > > > > > > Signed-off-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com&gt; > > > > --- > > > > include/linux/skbuff.h | 24 +++++++++++++++++++++- > > > > net/core/skbuff.c | 56 ++++++++++++++++++++++++++++++++++++++------- > > > ----- > > > > 2 files changed, 66 insertions(+), 14 deletions(-) > > > > > > Boris, below is the change I mentioned to you. > > > > > > It allows to allocate 48 additional bytes on-demand after skb_shared_info. > > > As it is on-demand, it won't increase the size of the skb for other users. > > > > > > For example, TLS could start using it when it creates the skb that it > > > pushes down to the TCP-stack. That way you don't need to handle the > > > tls_record lists. > > > > > > > One small problem is that TLS doesn't create SKBs. As a ULP it calls the transport send > > Functions (do_tcp_sendpages for TLS). This function receives a page and not a SKB. > > yes, that's a good point. Mat has another patch as part of this series, > that adds an skb-arg to sendpages > (https://lists.01.org/pipermail/mptcp/2017-October/000130.html) > > That should do the job for you. After working with the extended control block some more, I found that the arg to sendpages (at least as I implemented it in that patch) doesn't work out because skb_entail() isn't called. I'm experimenting with allocating and entailing an empty skb before calling do_tcp_sendpages(), which will then find the extended skb on the write queue and append to it. It's involving a lot of code to handle the memory waits and error conditions, though - it's probably cleaner to plumb some parameters in to do_tcp_sendpages() and sk_stream_alloc_skb() to request the extended skb.