MPTCP Linux Development
 help / color / mirror / Atom feed
* [PATCH mptcp-next v2 0/5] prepare interfaces for kTLS and sockmap support
@ 2026-10-29  7:38 Geliang Tang
  2026-10-29  7:38 ` [PATCH mptcp-next v2 1/5] mptcp: update mptcp_check_readable Geliang Tang
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Geliang Tang @ 2026-10-29  7:38 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

v2:
 - drop the patch "prevent dummy DATA_FIN on RST in fallback mode".
 - keep mptcp_inq_hint in mptcp_ioctl unchanged.
 - use mptcp_data_avail in mptcp_inq.
 - add msk_owned_by_me check in mptcp_sendmsg_locked.

v1:
 - https://patchwork.kernel.org/project/mptcp/cover/cover.1792999988.git.tanggeliang@kylinos.cn/

This series was previously posted as part of "Reduce the differences
between TCP and MPTCP for TLS usage" v15. It creates the mptcp_inq()
and mptcp_sendmsg_locked() interfaces that are required for the upcoming
MPTCP kTLS and MPTCP sockmap implementations.

The series has been rebased and no longer depends on Paolo's CB layout
refactoring patches (mptcp: drop the mptcp_ooo_try_coalesce() helper,
mptcp: drop the cant_coalesce CB field, mptcp: remove CB offset field,
mptcp: sync mptcp skb cb layout with tcp one, mptcp: defer read_sock
cleanup to mptcp_worker). Those patches are more complex and will be
sent in a separate series.

Patch overview:

1. mptcp: update mptcp_check_readable
 - Align with TCP's sock_is_readable for better TLS compatibility

2. mptcp: prevent dummy DATA_FIN on RST in fallback mode
 - Fix semantic issue where RST on fallback connections would create
   dummy DATA_FIN, causing unnecessary state transitions
 - Add sk_err check to subflow_is_done() to distinguish abortive
   closes (RST) from clean closes (FIN)

3. mptcp: implement peek_len for proto_ops
 - Add mptcp_inq() to report readable bytes at MPTCP level
 - Wire into .peek_len proto_ops for both IPv4 and IPv6
 - Handle 32-bit torn reads by returning 0 instead of INT_MAX
 - Prepare for TLS strparser integration

4. mptcp: add sendmsg_locked to proto_ops
 - Split mptcp_sendmsg() into locked/unlocked variants
 - Expose .sendmsg_locked in proto_ops for TLS send path

5. mptcp: track app-limited state in mptcp_sendmsg
 - Add mptcp_rate_check_app_limited() to mark subflows as app-limited
 - Bail out when MPTCP has unsent data (write_seq != snd_nxt or
   mptcp_send_head() != NULL) to avoid false app-limited marking
 - Enable accurate delivery rate estimation for BBR

6. selftests: mptcp: sockopt: check app_limited
 - Add regression test for app-limited accounting
 - Verify tcpi_delivery_rate_app_limited == 1 after small transfers

Gang Yan (1):
  mptcp: update mptcp_check_readable

Geliang Tang (4):
  mptcp: implement peek_len for proto_ops
  mptcp: add sendmsg_locked to proto_ops
  mptcp: track app-limited state in mptcp_sendmsg
  selftests: mptcp: sockopt: check app_limited

 net/mptcp/protocol.c                          | 79 +++++++++++++++++--
 .../selftests/net/mptcp/mptcp_sockopt.c       |  5 ++
 2 files changed, 77 insertions(+), 7 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next v2 2/5] mptcp: implement peek_len for proto_ops
@ 2026-10-30 22:02 netdev-bot+sashiko
  0 siblings, 0 replies; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-10-30 22:02 UTC (permalink / raw)
  To: geliang; +Cc: mptcp, tanggeliang, kuba

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] mptcp_inq() (net/mptcp/protocol.c, lines 4815-4817) decrements
  the count whenever msk->rcv_data_fin is set: `if (answ &&…
- [Low] The commit message's rationale and comparison don't match the
  tree.

--

mptcp: implement peek_len for proto_ops

This commit adds mptcp_inq(), which computes the readable bytes at the
MPTCP level from mptcp_data_avail(). It subtracts one byte when
msk->rcv_data_fin is set. The new mptcp_peek_len() wraps it and is set
as .peek_len in mptcp_stream_ops and mptcp_v6_stream_ops.

> READ_ONCE() is used instead of mptcp_data_lock() because mptcp_inq()
> will be called from TLS contexts with socket lock held - taking the
> spinlock would deadlock.

[ ... ]

> Note: mptcp_inq() differs from mptcp_inq_hint() by returning 0 (not 1)
> when value is 0, and using mptcp_data_avail() with rcv_data_fin check
> instead of ack_seq - copied_seq with TCP_CLOSE/RCV_SHUTDOWN check, to
> correctly handle TFO scenarios.

[Severity: Low]
Which TLS path does the first paragraph refer to? mptcp_inq() is static
and can only be reached through ->peek_len. TLS calls tcp_inq() directly,
for example in tls_strp_read_sock():

	inq = tcp_inq(strp->sk);

The existing ->peek_len callers can't reach MPTCP sockets either:

- kcm_attach() rejects sockets whose sk_protocol isn't IPPROTO_TCP.
- sock_map only sets up strparser when sk_is_tcp() is true.
- vhost_net only uses tun/tap.

So the new .peek_len has no consumer in the tree. If a later series is
meant to use it, could the commit message name that series?

The comparison with mptcp_inq_hint() also doesn't seem to match the
code. mptcp_inq_hint() subtracts the head skb's map_seq from ack_seq,
not copied_seq. It returns 1 only when the receive queue is empty and
the socket is closed or shut down for receive:

net/mptcp/protocol.c:mptcp_inq_hint() {
	...
	skb = skb_peek(&sk->sk_receive_queue);
	if (skb) {
		u64 hint_val = READ_ONCE(msk->ack_seq) - MPTCP_SKB_CB(skb)->map_seq;
	...
	if (sk->sk_state == TCP_CLOSE || (sk->sk_shutdown & RCV_SHUTDOWN))
		return 1;

	return 0;
}

Could this note be updated to describe the current mptcp_inq_hint()?

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 0caa31525b907..6f34e3459e8ac 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -4797,6 +4797,33 @@ static ssize_t mptcp_splice_read(struct socket *sock, loff_t *ppos,
>  	return ret;
>  }
>  
> +static int mptcp_inq(struct sock *sk)
> +{
> +	const struct mptcp_sock *msk = mptcp_sk(sk);
> +	u64 avail;
> +	int answ;
> +
> +	if ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV))
> +		return 0;
> +
> +	avail = mptcp_data_avail(msk);
> +	if (avail >= INT_MAX)
> +		return 0;
> +
> +	answ = (unsigned int)avail;
> +
> +	/* Subtract 1, if DATA_FIN was received. Mirror tcp_inq() */
> +	if (answ && READ_ONCE(msk->rcv_data_fin))
> +		answ--;

[Severity: Medium]
Does this decrement actually mirror tcp_inq()? In tcp_inq(), rcv_nxt
includes the FIN's sequence slot and SOCK_DONE stays set, so the -1
removes that slot:

include/net/tcp.h:tcp_inq() {
	...
		answ = tp->rcv_nxt - tp->copied_seq;

		/* Subtract 1, if FIN was received */
		if (answ && sock_flag(sk, SOCK_DONE))
			answ--;
	...
}

mptcp_data_avail() is bytes_received - bytes_consumed. bytes_received
only counts payload, for example "+= copy_len" in __mptcp_move_skb() and
"+= skb->len" in fastopen.c. get_mapping_status() also removes the
DATA_FIN byte from the mapping:

net/mptcp/subflow.c:get_mapping_status() {
	...
		/* Adjust for DATA_FIN using 1 byte of sequence space */
		data_len--;
	...
}

The DATA_FIN is only counted in msk->ack_seq, and rcv_data_fin is
cleared in the same step:

net/mptcp/protocol.c:mptcp_check_data_fin() {
	...
	if (mptcp_pending_data_fin(sk, &rcv_data_fin_seq)) {
		WRITE_ONCE(msk->ack_seq, msk->ack_seq + 1);
		WRITE_ONCE(msk->rcv_data_fin, 0);
	...
}

So rcv_data_fin == 1 seems to mean the DATA_FIN has been parsed but not
counted anywhere yet. That is exactly when there is no FIN byte to
subtract.

Wouldn't mptcp_peek_len() then return N-1 instead of N, or 0 instead of
1, whenever this branch runs?

The window looks reachable. Say the peer sends its last N bytes with
DATA_FIN in the same DSS. mptcp_update_rcv_data_fin() sets rcv_data_fin
at option parse time, from subflow context. move_skbs_to_msk() then
moves the data and only schedules the worker:

net/mptcp/protocol.c:move_skbs_to_msk() {
	...
	if (mptcp_pending_data_fin(sk, NULL))
		mptcp_schedule_work(sk);
	...
}

sk_data_ready() then runs while rcv_data_fin is still set. If the
DATA_FIN arrives ahead of a gap in the data sequence, the flag stays set
longer, and earlier in-sequence data is under-reported too.

A strparser-style ->peek_len consumer would see a final record that ends
at the DATA_FIN as incomplete in strp_data_ready():

	if (strp_peek_len(strp) < strp->need_bytes)
		return;

When the DATA_FIN is processed later, mptcp_close_wake_up() only calls
sk_state_change() and sk_wake_async(), not sk_data_ready(). Nothing would
re-trigger the parser.

No ->peek_len caller in the current tree reaches MPTCP sockets, so this
downstream effect depends on a future consumer. The code is the same at
the end of the series.

mptcp_data_avail() already leaves out the DATA_FIN. Should the
rcv_data_fin adjustment be dropped?

> +
> +	return answ;
> +}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1793258749.git.tanggeliang%40kylinos.cn

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-10-30 22:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-29  7:38 [PATCH mptcp-next v2 0/5] prepare interfaces for kTLS and sockmap support Geliang Tang
2026-10-29  7:38 ` [PATCH mptcp-next v2 1/5] mptcp: update mptcp_check_readable Geliang Tang
2026-10-29  7:38 ` [PATCH mptcp-next v2 2/5] mptcp: implement peek_len for proto_ops Geliang Tang
2026-10-29  7:52   ` sashiko-bot
2026-10-29  7:38 ` [PATCH mptcp-next v2 3/5] mptcp: add sendmsg_locked to proto_ops Geliang Tang
2026-10-29  7:38 ` [PATCH mptcp-next v2 4/5] mptcp: track app-limited state in mptcp_sendmsg Geliang Tang
2026-10-29  7:38 ` [PATCH mptcp-next v2 5/5] selftests: mptcp: sockopt: check app_limited Geliang Tang
2026-10-29  9:39 ` [PATCH mptcp-next v2 0/5] prepare interfaces for kTLS and sockmap support MPTCP CI
  -- strict thread matches above, loose matches on Subject: below --
2026-10-30 22:02 [PATCH mptcp-next v2 2/5] mptcp: implement peek_len for proto_ops netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox