[MPTCP] Re: [PATCH mptcp-next 3/7] mptcp: avoid blocking in tcp_sendpages due to skb alloc

[Florian Westphal <fw at strlen.de>]

[-- Attachment #1: Type: text/plain, Size: 1927 bytes --]

Paolo Abeni <pabeni(a)redhat.com> wrote:
> On Wed, 2020-05-06 at 15:02 +0200, Florian Westphal wrote:
> > If memory is tight, tcp_sendpages may fail because it can't
> > allocate the skb head.
> > 
> > Now that we pass DONTWAIT to tcp_sendpages it won't block anymore on the
> > subflow socket when this happens.
> > 
> > We can avoid such allocation failure in tcp code by preallocating an skb
> > skb when we pick the subflow to use.
> 
> The sk_tx_skb_cache usage caused some TCP regression upstream:
> 
> commit 0b7d7f6b22084a3156f267c85303908a8f4c9a08
> Author: Eric Dumazet <edumazet(a)google.com>
> Date:   Fri Jun 14 16:22:20 2019 -0700
> 
>     tcp: add tcp_tx_skb_cache sysctl
> 
> but that one was not very clear and I guess does not matter for MPTCP -
> lacking an established baseline.
> 
> So all good ;)
> 
> > @@ -656,9 +677,13 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
> >  static struct sock *mptcp_subflow_get_send(struct mptcp_sock *msk)
> >  {
> >  	struct mptcp_subflow_context *subflow;
> > +	struct sock *sk = (struct sock *)msk;
> >  	struct sock *backup = NULL;
> >  
> > -	sock_owned_by_me((const struct sock *)msk);
> > +	sock_owned_by_me(sk);
> > +
> > +	if (!mptcp_sendmsg_alloc_skb(sk))
> > +		return NULL;
> >  
> >  	mptcp_for_each_subflow(msk, subflow) {
> >  		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> 
> Don't we need the same for mptcp_subflow_get_retrans() ?

Its optional.  If allocation doesn't fail there is no difference.

If it fails then as-is, mptcp_sendmsg_frag will return -EAGAIN and
no packet is sent.

If I'd add mptcp_sendmsg_alloc_skb() call to the worker, then it
would bail our right before (and would not sleep either).

So the end result is the same: no retransmission takes place.

Let me know if you prefer explicit mptcp_sendmsg_alloc_skb()
for retransmit case too.

Thanks!