[PATCH mptcp-net] mptcp: try harder to borrow memory from subflow under pressure

[PATCH mptcp-net] mptcp: try harder to borrow memory from subflow under pressure

[Paolo Abeni]

If the host is under sever memory pressure, and RX forward
memory allocation for the msk fails, we try to borrow the
required memory from the ingress subflow.

The current attempt is a bit flaky: if skb->truesize is less
than SK_MEM_QUANTUM, the ssk will not release any memory, and
the next schedule will fail again.

Instead, directly move the required amount of pages from the
ssk to the msk, if available

Fixes: 9c3f94e1681b ("mptcp: add missing memory scheduling in the rx path")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index bb029dd4ff5e..785e74c13b3c 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -286,11 +286,13 @@ static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk,
 
 	/* try to fetch required memory from subflow */
 	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
-		if (ssk->sk_forward_alloc < skb->truesize)
-			goto drop;
-		__sk_mem_reclaim(ssk, skb->truesize);
-		if (!sk_rmem_schedule(sk, skb, skb->truesize))
+		int amount = sk_mem_pages(skb->truesize) << SK_MEM_QUANTUM_SHIFT;
+
+		if (ssk->sk_forward_alloc < amount)
 			goto drop;
+
+		ssk->sk_forward_alloc -= amount;
+		sk->sk_forward_alloc += amount;
 	}
 
 	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
-- 
2.26.3

Re: [PATCH mptcp-net] mptcp: try harder to borrow memory from subflow under pressure

[Mat Martineau]

On Wed, 26 May 2021, Paolo Abeni wrote:

> If the host is under sever memory pressure, and RX forward
> memory allocation for the msk fails, we try to borrow the
> required memory from the ingress subflow.
>
> The current attempt is a bit flaky: if skb->truesize is less
> than SK_MEM_QUANTUM, the ssk will not release any memory, and
> the next schedule will fail again.
>
> Instead, directly move the required amount of pages from the
> ssk to the msk, if available
>
> Fixes: 9c3f94e1681b ("mptcp: add missing memory scheduling in the rx path")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/mptcp/protocol.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index bb029dd4ff5e..785e74c13b3c 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -286,11 +286,13 @@ static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk,
>
> 	/* try to fetch required memory from subflow */
> 	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
> -		if (ssk->sk_forward_alloc < skb->truesize)
> -			goto drop;
> -		__sk_mem_reclaim(ssk, skb->truesize);
> -		if (!sk_rmem_schedule(sk, skb, skb->truesize))
> +		int amount = sk_mem_pages(skb->truesize) << SK_MEM_QUANTUM_SHIFT;
> +
> +		if (ssk->sk_forward_alloc < amount)
> 			goto drop;
> +
> +		ssk->sk_forward_alloc -= amount;
> +		sk->sk_forward_alloc += amount;
> 	}
>
> 	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
> -- 
> 2.26.3

Looks ok to me - the memory will be reclaimed from the msk later, if I 
understand correctly.

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

--
Mat Martineau
Intel

Re: [PATCH mptcp-net] mptcp: try harder to borrow memory from subflow under pressure

[Paolo Abeni]

On Wed, 2021-05-26 at 21:17 -0700, Mat Martineau wrote:
> On Wed, 26 May 2021, Paolo Abeni wrote:
> 
> > If the host is under sever memory pressure, and RX forward
> > memory allocation for the msk fails, we try to borrow the
> > required memory from the ingress subflow.
> > 
> > The current attempt is a bit flaky: if skb->truesize is less
> > than SK_MEM_QUANTUM, the ssk will not release any memory, and
> > the next schedule will fail again.
> > 
> > Instead, directly move the required amount of pages from the
> > ssk to the msk, if available
> > 
> > Fixes: 9c3f94e1681b ("mptcp: add missing memory scheduling in the rx path")
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> > net/mptcp/protocol.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> > index bb029dd4ff5e..785e74c13b3c 100644
> > --- a/net/mptcp/protocol.c
> > +++ b/net/mptcp/protocol.c
> > @@ -286,11 +286,13 @@ static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk,
> > 
> > 	/* try to fetch required memory from subflow */
> > 	if (!sk_rmem_schedule(sk, skb, skb->truesize)) {
> > -		if (ssk->sk_forward_alloc < skb->truesize)
> > -			goto drop;
> > -		__sk_mem_reclaim(ssk, skb->truesize);
> > -		if (!sk_rmem_schedule(sk, skb, skb->truesize))
> > +		int amount = sk_mem_pages(skb->truesize) << SK_MEM_QUANTUM_SHIFT;
> > +
> > +		if (ssk->sk_forward_alloc < amount)
> > 			goto drop;
> > +
> > +		ssk->sk_forward_alloc -= amount;
> > +		sk->sk_forward_alloc += amount;
> > 	}
> > 
> > 	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
> > -- 
> > 2.26.3
> 
> Looks ok to me - the memory will be reclaimed from the msk later, if I 
> understand correctly.

Yes, sk_forward_alloc will be updated when the skb will be freed at
reception time, and the fwd allocated memory is reclaimed either on
memory pressure or at socket close time. 
> 
> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

Thanks!

Paolo

Re: [PATCH mptcp-net] mptcp: try harder to borrow memory from subflow under pressure

[Matthieu Baerts]

Hi Paolo, Mat,

On 26/05/2021 18:10, Paolo Abeni wrote:
> If the host is under sever memory pressure, and RX forward
> memory allocation for the msk fails, we try to borrow the
> required memory from the ingress subflow.
> 
> The current attempt is a bit flaky: if skb->truesize is less
> than SK_MEM_QUANTUM, the ssk will not release any memory, and
> the next schedule will fail again.
> 
> Instead, directly move the required amount of pages from the
> ssk to the msk, if available
> 
> Fixes: 9c3f94e1681b ("mptcp: add missing memory scheduling in the rx path")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Thank you for the patch and the review! (and sorry for the delay)

- fa92849db4ec: mptcp: try harder to borrow memory from subflow under
pressure
- Results: 809268f427a5..119541ecd3c9

Builds and tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20210604T193342
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export/20210604T193342

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net