linux-kernel - Re: [PATCH] nvme/tcp: handle tls partially sent records in write

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <339cbb66fbcd78d639d0d8463a3a67daf089f40d.camel@gmail.com>
Date: Tue, 07 Oct 2025 19:24:53 +1000
From: Wilfred Mallawa <wilfred.opensource@...il.com>
To: Hannes Reinecke <hare@...e.de>, linux-nvme@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Cc: Keith Busch <kbusch@...nel.org>, Jens Axboe <axboe@...nel.dk>, Christoph
 Hellwig <hch@....de>, Sagi Grimberg <sagi@...mberg.me>, John Fastabend
 <john.fastabend@...il.com>,  Jakub Kicinski	 <kuba@...nel.org>, Sabrina
 Dubroca <sd@...asysnail.net>, "David S . Miller"	 <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Paolo Abeni	 <pabeni@...hat.com>, Simon
 Horman <horms@...nel.org>
Subject: Re: [PATCH] nvme/tcp: handle tls partially sent records in
 write_space()

On Tue, 2025-10-07 at 07:19 +0200, Hannes Reinecke wrote:
> On 10/7/25 02:46, Wilfred Mallawa wrote:
> > From: Wilfred Mallawa <wilfred.mallawa@....com>
> > 
> 
[...]
> I wonder: Do we really need to check for a partially assembled
> record,
> or wouldn't it be easier to call queue->write_space() every time
> here?
> We sure would end up with executing the callback more often, but if
> no
> data is present it shouldn't do any harm.
> 
> IE just use
> 
> if (nvme_tcp_queue_tls(queue)
>      queue->write_space(sk);

Hey Hannes,

This was my initial approach, but I figured using
tls_is_partially_sent_record() might be slightly more efficient. But if
we think that's negligible, happy to go with this approach (omitting
the partial record check).

Wilfred

> 
> > @@ -1306,6 +1313,7 @@ static int nvme_tcp_try_send_ddgst(struct
> > nvme_tcp_request *req)
> >   static int nvme_tcp_try_send(struct nvme_tcp_queue *queue)
> >   {
> >   	struct nvme_tcp_request *req;
> > +	struct tls_context *ctx = tls_get_ctx(queue->sock->sk);
> >   	unsigned int noreclaim_flag;
> >   	int ret = 1;
> >   And we need this why?
> 
> > diff --git a/include/net/tls.h b/include/net/tls.h
> > index 857340338b69..9c61a2de44bf 100644
> > --- a/include/net/tls.h
> > +++ b/include/net/tls.h
> > @@ -373,6 +373,11 @@ static inline struct tls_context
> > *tls_get_ctx(const struct sock *sk)
> >   	return (__force void *)icsk->icsk_ulp_data;
> >   }
> >   
> > +static inline bool tls_is_partially_sent_record(struct tls_context
> > *ctx)
> > +{
> > +	return !!ctx->partially_sent_record;
> > +}
> > +
> >   static inline struct tls_sw_context_rx *tls_sw_ctx_rx(
> >   		const struct tls_context *tls_ctx)
> >   {
> > diff --git a/net/tls/tls.h b/net/tls/tls.h
> > index 2f86baeb71fc..7839a2effe31 100644
> > --- a/net/tls/tls.h
> > +++ b/net/tls/tls.h
> > @@ -271,11 +271,6 @@ int tls_push_partial_record(struct sock *sk,
> > struct tls_context *ctx,
> >   			    int flags);
> >   void tls_free_partial_record(struct sock *sk, struct tls_context
> > *ctx);
> >   
> > -static inline bool tls_is_partially_sent_record(struct tls_context
> > *ctx)
> > -{
> > -	return !!ctx->partially_sent_record;
> > -}
> > -
> >   static inline bool tls_is_pending_open_record(struct tls_context
> > *tls_ctx)
> >   {
> >   	return tls_ctx->pending_open_record_frags;
> See above. If we were calling ->write_space unconditionally we 
> wouldn'teven need this export.Cheers,Hannes