[PATCH mptcp-next 0/3] mptcp: tx refactor update

[PATCH mptcp-next 0/3] mptcp: tx refactor update

[Paolo Abeni]

This are updates to the tx refactor patches as asked by Eric.
Note that patch 3/3 should replace completely the existing
'Partially revert "tcp: factor out tcp_build_frag()"'
patch. I opted for this solution as the git repo mangling looked
easier this way; @matttbe: please tell me if other options are
preferred instead.

Additionally patch "chtls: rename skb_entail() to chtls_skb_entail()"
should be dropped.


 include/net/tcp.h | 2 +-
 net/ipv4/tcp.c    | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.26.3

[PATCH mptcp-next 1/3] Squash-to: "tcp: expose the tcp_mark_push() and skb_entail() helpers"

[Paolo Abeni]

Also change the commit title and message to:

"""
tcp: expose the tcp_mark_push() and tcp_skb_entail() helpers

the tcp_skb_entail() helper actually skb_entail(), renamed
to provide proper scope.

    The two helper will be used by the next patch.

RFC -> v1:
 - rename skb_entail to tcp_skb_entail (Eric)

"""

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 include/net/tcp.h | 2 +-
 net/ipv4/tcp.c    | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index dc52ea8adfc7..4a96f6e0f6f8 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -581,7 +581,7 @@ __u32 cookie_v6_init_sequence(const struct sk_buff *skb, __u16 *mss);
 #endif
 /* tcp_output.c */
 
-void skb_entail(struct sock *sk, struct sk_buff *skb);
+void tcp_skb_entail(struct sock *sk, struct sk_buff *skb);
 void tcp_mark_push(struct tcp_sock *tp, struct sk_buff *skb);
 void __tcp_push_pending_frames(struct sock *sk, unsigned int cur_mss,
 			       int nonagle);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 7a3e632b0048..c592454625e1 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -658,7 +658,7 @@ static inline bool forced_push(const struct tcp_sock *tp)
 	return after(tp->write_seq, tp->pushed_seq + (tp->max_window >> 1));
 }
 
-void skb_entail(struct sock *sk, struct sk_buff *skb)
+void tcp_skb_entail(struct sock *sk, struct sk_buff *skb)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct tcp_skb_cb *tcb = TCP_SKB_CB(skb);
@@ -985,7 +985,7 @@ struct sk_buff *tcp_build_frag(struct sock *sk, int size_goal, int flags,
 #ifdef CONFIG_TLS_DEVICE
 		skb->decrypted = !!(flags & MSG_SENDPAGE_DECRYPTED);
 #endif
-		skb_entail(sk, skb);
+		tcp_skb_entail(sk, skb);
 		copy = size_goal;
 	}
 
@@ -1314,7 +1314,7 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
 			process_backlog++;
 			skb->ip_summed = CHECKSUM_PARTIAL;
 
-			skb_entail(sk, skb);
+			tcp_skb_entail(sk, skb);
 			copy = size_goal;
 
 			/* All packets are restored as if they have
-- 
2.26.3

[PATCH mptcp-next 2/3] Squash-to: "mptcp: stop relying on tcp_tx_skb_cache"

[Paolo Abeni]

fix build after helper rename

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 48e0e3a945e4..87ee409d68ab 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1240,7 +1240,7 @@ static struct sk_buff *__mptcp_alloc_tx_skb(struct sock *sk, struct sock *ssk, g
 		return NULL;
 
 	if (likely(sk_wmem_schedule(ssk, skb->truesize))) {
-		skb_entail(ssk, skb);
+		tcp_skb_entail(ssk, skb);
 		return skb;
 	}
 	kfree_skb(skb);
-- 
2.26.3

[PATCH mptcp-next 3/3] tcp: make tcp_build_frag() static

[Paolo Abeni]

After the previous patch the mentioned helper is
used only inside its compilation unit: let's make
it static.

RFC -> v1:
 - preserve the tcp_build_frag() helper (Eric)

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
Note:
this should _replace_:
"""
Partially revert "tcp: factor out tcp_build_frag()"
"""

I did not opted for a squash to patch, as it would
still require conflicts resolution after the previous
squash-to patch. @matttbe: please let me know if you
prefer otherwise.
---
 include/net/tcp.h | 2 --
 net/ipv4/tcp.c    | 5 ++---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 4a96f6e0f6f8..673c3b01e287 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -330,8 +330,6 @@ int tcp_sendpage(struct sock *sk, struct page *page, int offset, size_t size,
 		 int flags);
 int tcp_sendpage_locked(struct sock *sk, struct page *page, int offset,
 			size_t size, int flags);
-struct sk_buff *tcp_build_frag(struct sock *sk, int size_goal, int flags,
-			       struct page *page, int offset, size_t *size);
 ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
 		 size_t size, int flags);
 int tcp_send_mss(struct sock *sk, int *size_goal, int flags);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index c592454625e1..4ae43222d71f 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -963,8 +963,8 @@ void tcp_remove_empty_skb(struct sock *sk, struct sk_buff *skb)
 	}
 }
 
-struct sk_buff *tcp_build_frag(struct sock *sk, int size_goal, int flags,
-			       struct page *page, int offset, size_t *size)
+static struct sk_buff *tcp_build_frag(struct sock *sk, int size_goal, int flags,
+				      struct page *page, int offset, size_t *size)
 {
 	struct sk_buff *skb = tcp_write_queue_tail(sk);
 	struct tcp_sock *tp = tcp_sk(sk);
@@ -1064,7 +1064,6 @@ ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
 		size_t copy = size;
 
 		skb = tcp_build_frag(sk, size_goal, flags, page, offset, &copy);
-		if (!skb)
 			goto wait_for_space;
 
 		if (!copied)
-- 
2.26.3

Re: [PATCH mptcp-next 3/3] tcp: make tcp_build_frag() static

[Mat Martineau]

On Mon, 20 Sep 2021, Paolo Abeni wrote:

> After the previous patch the mentioned helper is
> used only inside its compilation unit: let's make
> it static.
>
> RFC -> v1:
> - preserve the tcp_build_frag() helper (Eric)
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> Note:
> this should _replace_:
> """
> Partially revert "tcp: factor out tcp_build_frag()"
> """
>
> I did not opted for a squash to patch, as it would
> still require conflicts resolution after the previous
> squash-to patch. @matttbe: please let me know if you
> prefer otherwise.
> ---
> include/net/tcp.h | 2 --
> net/ipv4/tcp.c    | 5 ++---
> 2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 4a96f6e0f6f8..673c3b01e287 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -330,8 +330,6 @@ int tcp_sendpage(struct sock *sk, struct page *page, int offset, size_t size,
> 		 int flags);
> int tcp_sendpage_locked(struct sock *sk, struct page *page, int offset,
> 			size_t size, int flags);
> -struct sk_buff *tcp_build_frag(struct sock *sk, int size_goal, int flags,
> -			       struct page *page, int offset, size_t *size);
> ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
> 		 size_t size, int flags);
> int tcp_send_mss(struct sock *sk, int *size_goal, int flags);
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index c592454625e1..4ae43222d71f 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -963,8 +963,8 @@ void tcp_remove_empty_skb(struct sock *sk, struct sk_buff *skb)
> 	}
> }
>
> -struct sk_buff *tcp_build_frag(struct sock *sk, int size_goal, int flags,
> -			       struct page *page, int offset, size_t *size)
> +static struct sk_buff *tcp_build_frag(struct sock *sk, int size_goal, int flags,
> +				      struct page *page, int offset, size_t *size)
> {
> 	struct sk_buff *skb = tcp_write_queue_tail(sk);
> 	struct tcp_sock *tp = tcp_sk(sk);
> @@ -1064,7 +1064,6 @@ ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
> 		size_t copy = size;
>
> 		skb = tcp_build_frag(sk, size_goal, flags, page, offset, &copy);
> -		if (!skb)
> 			goto wait_for_space;

This looks like an accidental deletion. Do you want to repost or have 
Matthieu drop this hunk?

--
Mat Martineau
Intel

Re: [PATCH mptcp-next 0/3] mptcp: tx refactor update

[Mat Martineau]

On Mon, 20 Sep 2021, Paolo Abeni wrote:

> This are updates to the tx refactor patches as asked by Eric.
> Note that patch 3/3 should replace completely the existing
> 'Partially revert "tcp: factor out tcp_build_frag()"'
> patch. I opted for this solution as the git repo mangling looked
> easier this way; @matttbe: please tell me if other options are
> preferred instead.
>
> Additionally patch "chtls: rename skb_entail() to chtls_skb_entail()"
> should be dropped.
>
>
> include/net/tcp.h | 2 +-
> net/ipv4/tcp.c    | 6 +++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>

Patches 1 & 2 are ok to squash, patch 3 has an issue (see separate reply).

--
Mat Martineau
Intel

Re: [PATCH mptcp-next 3/3] tcp: make tcp_build_frag() static

[Paolo Abeni]

On Mon, 2021-09-20 at 15:02 -0700, Mat Martineau wrote:
> On Mon, 20 Sep 2021, Paolo Abeni wrote:
> 
> > After the previous patch the mentioned helper is
> > used only inside its compilation unit: let's make
> > it static.
> > 
> > RFC -> v1:
> > - preserve the tcp_build_frag() helper (Eric)
> > 
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> > Note:
> > this should _replace_:
> > """
> > Partially revert "tcp: factor out tcp_build_frag()"
> > """
> > 
> > I did not opted for a squash to patch, as it would
> > still require conflicts resolution after the previous
> > squash-to patch. @matttbe: please let me know if you
> > prefer otherwise.
> > ---
> > include/net/tcp.h | 2 --
> > net/ipv4/tcp.c    | 5 ++---
> > 2 files changed, 2 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/net/tcp.h b/include/net/tcp.h
> > index 4a96f6e0f6f8..673c3b01e287 100644
> > --- a/include/net/tcp.h
> > +++ b/include/net/tcp.h
> > @@ -330,8 +330,6 @@ int tcp_sendpage(struct sock *sk, struct page *page, int offset, size_t size,
> > 		 int flags);
> > int tcp_sendpage_locked(struct sock *sk, struct page *page, int offset,
> > 			size_t size, int flags);
> > -struct sk_buff *tcp_build_frag(struct sock *sk, int size_goal, int flags,
> > -			       struct page *page, int offset, size_t *size);
> > ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
> > 		 size_t size, int flags);
> > int tcp_send_mss(struct sock *sk, int *size_goal, int flags);
> > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> > index c592454625e1..4ae43222d71f 100644
> > --- a/net/ipv4/tcp.c
> > +++ b/net/ipv4/tcp.c
> > @@ -963,8 +963,8 @@ void tcp_remove_empty_skb(struct sock *sk, struct sk_buff *skb)
> > 	}
> > }
> > 
> > -struct sk_buff *tcp_build_frag(struct sock *sk, int size_goal, int flags,
> > -			       struct page *page, int offset, size_t *size)
> > +static struct sk_buff *tcp_build_frag(struct sock *sk, int size_goal, int flags,
> > +				      struct page *page, int offset, size_t *size)
> > {
> > 	struct sk_buff *skb = tcp_write_queue_tail(sk);
> > 	struct tcp_sock *tp = tcp_sk(sk);
> > @@ -1064,7 +1064,6 @@ ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
> > 		size_t copy = size;
> > 
> > 		skb = tcp_build_frag(sk, size_goal, flags, page, offset, &copy);
> > -		if (!skb)
> > 			goto wait_for_space;
> 
> This looks like an accidental deletion. Do you want to repost or have 
> Matthieu drop this hunk?

whoops, thanks for noticing! I'll repost.

Cheers,

Paolo