Re: [PATCH next mptcp-net 3/3] mptcp: remove thmac from subflow ctx

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 19/08/2026 11:56, Geliang Tang wrote:
> Hi Matt,
> 
> On Tue, 2026-08-18 at 19:55 +0200, Matthieu Baerts (NGI0) wrote:
>> This entry is only used in subflow_finish_connect().
>>
>> Instead, use the original value from mp_opt, and pass it to
>> subflow_thmac_valid() to do the validation with the given truncated
>> hmac.
>>
>> While at it, rename the variables in subflow_thmac_valid() to avoid
>> confusions about the received one vs the expected one.
>>
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>>  net/mptcp/protocol.h |  1 -
>>  net/mptcp/subflow.c  | 18 +++++++++---------
>>  2 files changed, 9 insertions(+), 10 deletions(-)
>>
>> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
>> index 7e168e450fb0..d414065d1966 100644
>> --- a/net/mptcp/protocol.h
>> +++ b/net/mptcp/protocol.h
>> @@ -592,7 +592,6 @@ struct mptcp_subflow_context {
>>  	bool	fully_established;  /* path validated */
>>  	u32	lent_mem_frag;
>>  	u32	remote_nonce;
>> -	u64	thmac;
>>  	u32	local_nonce;
>>  	u32	remote_token;
>>  	union {
>> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
>> index 01db7edce18a..2d7ccb01d234 100644
>> --- a/net/mptcp/subflow.c
>> +++ b/net/mptcp/subflow.c
>> @@ -408,20 +408,21 @@ static struct dst_entry
>> *subflow_v6_route_req(const struct sock *sk,
>>  #endif
>>  
>>  /* validate received truncated hmac and create hmac for third ACK */
>> -static bool subflow_thmac_valid(struct mptcp_subflow_context
>> *subflow)
>> +static bool subflow_thmac_valid(struct mptcp_subflow_context
>> *subflow,
>> +				u64 thmac)
> 
> Perhaps we could rename this parameter, say to "subflow_thmac", so that
> we don't need to rename the local variable "thmac" to "expected_thmac".
> What do you think?
I initially did that, but it fell more natural to use the "expected"
keyword, similar to other places, and in the selftests, than the
opposite. Also, we pass "thmac" to subflow_thmac_valid(), but seeing the
argument renamed didn't feel right.

If it was for -net, I would have minimised the diff, but here, let's get
thing rights I think, no?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.