Re: [PATCH] nvme/tcp: handle tls partially sent records in write_space()

[Wilfred Mallawa <wilfred.opensource@gmail.com>]

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@wdc.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