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

[Geliang Tang <geliang@kernel.org>]

Hi Matt,

On Wed, 2026-08-19 at 12:02 +0200, Matthieu Baerts wrote:
> 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?

OK, no further comments from my side. I'll add my Reviewed-by tag in
reply to the cover letter.

Thanks,
-Geliang

> 
> Cheers,
> Matt