write_space and software kTLS

write_space and software kTLS

[Chuck Lever III]

Hey Trond-

I've made some progress getting RPC-with-TLS working in
the Linux NFS client, but I recently hit an interesting
snag and could use a little advice.

The software kTLS infrastructure uses do_tcp_sendpages()
under the covers, and that function is clearing
SOCKWQ_ASYNC_NOSPACE from under xs_nospace(). That
prevents xs_run_error_worker() from waking up xprt->sending,
stalling an RPC transport waiting for more socket write
space. I'm not sure how to address this, and I'm interested
in your opinion.

For example, why check that flag rather than just waking
up xprt->sending unconditionally?

Also just for my own understanding of how the write_space
mechanism is supposed to work for RPC, I instrumented the
code that bumps and decrements sk_write_pending, and found
that under normal workloads, the value of that field goes
negative and stays there. I'm not sure that's intended...?


--
Chuck Lever

Re: write_space and software kTLS

[Trond Myklebust]

On Mon, 2022-03-14 at 21:35 +0000, Chuck Lever III wrote:
> Hey Trond-
> 
> I've made some progress getting RPC-with-TLS working in
> the Linux NFS client, but I recently hit an interesting
> snag and could use a little advice.
> 
> The software kTLS infrastructure uses do_tcp_sendpages()
> under the covers, and that function is clearing
> SOCKWQ_ASYNC_NOSPACE from under xs_nospace(). That
> prevents xs_run_error_worker() from waking up xprt->sending,
> stalling an RPC transport waiting for more socket write
> space. I'm not sure how to address this, and I'm interested
> in your opinion.
> 

How is it achieving this? We only set SOCKWQ_ASYNC_NOSPACE after the
call to xprt_sock_sendmsg().

> For example, why check that flag rather than just waking
> up xprt->sending unconditionally?

The socket code calls ->write_space() in all sorts of situations, so we
need to distinguish between the cases where we are actually waiting for
buffer memory, and the situations where we are not. Otherwise, we'd be
calling xs_run_error_worker() all the time.

> 
> Also just for my own understanding of how the write_space
> mechanism is supposed to work for RPC, I instrumented the
> code that bumps and decrements sk_write_pending, and found
> that under normal workloads, the value of that field goes
> negative and stays there. I'm not sure that's intended...?
> 

It is not intended, no. Looks like there have been various refactorings
that have mangled that code. From what I can see, the socket code
assumes that sk_write_pending should always be bumped with the
sock_lock() held.

Let me write a fix for that...


-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com

Re: write_space and software kTLS

[Chuck Lever III]

> On Mar 14, 2022, at 8:06 PM, Trond Myklebust <trondmy@hammerspace.com> wrote:
> 
> On Mon, 2022-03-14 at 21:35 +0000, Chuck Lever III wrote:
>> Hey Trond-
>> 
>> I've made some progress getting RPC-with-TLS working in
>> the Linux NFS client, but I recently hit an interesting
>> snag and could use a little advice.
>> 
>> The software kTLS infrastructure uses do_tcp_sendpages()
>> under the covers, and that function is clearing
>> SOCKWQ_ASYNC_NOSPACE from under xs_nospace(). That
>> prevents xs_run_error_worker() from waking up xprt->sending,
>> stalling an RPC transport waiting for more socket write
>> space. I'm not sure how to address this, and I'm interested
>> in your opinion.
>> 
> 
> How is it achieving this? We only set SOCKWQ_ASYNC_NOSPACE after the
> call to xprt_sock_sendmsg().

A kworker is clearing NOSPACE between the time xs_tcp_send_request()
sets it and the time xs_write_space() runs.

  kworker/u128:2-33    [003]   155.723869: rpc_socket_nospace:   task:000006cb@00000003 total=262380 remaining=131308
  kworker/u128:2-33    [003]   155.723870: bprint:               xs_nospace: sk=0xffff88810a8f0a00 setting SOCKWQ_ASYNC_NOSPACE
  kworker/u128:2-33    [003]   155.723879: xprt_transmit:        task:000006cb@00000003 xid=0x8ab69e2e seqno=0 status=-11
  kworker/u128:2-33    [003]   155.723881: xprt_release_xprt:    task:000006cc@00000003 snd_task:ffffffff
     kworker/3:2-116   [003]   155.723885: bprint:               do_tcp_sendpages: sk=0xffff88810a8f0a00 clearing SOCKWQ_ASYNC_NOSPACE
  kworker/u128:2-33    [003]   155.723888: rpc_task_run_action:  task:000006cc@00000003 flags=ASYNC|MOVEABLE|NORTO|CRED_NOREF runstate=RUNNING|ACTIVE|NEED_XMIT|NEED_RECV status=-11 action=call_transmit_status
  kworker/u128:2-33    [003]   155.723889: rpc_task_run_action:  task:000006cc@00000003 flags=ASYNC|MOVEABLE|NORTO|CRED_NOREF runstate=RUNNING|ACTIVE|NEED_XMIT|NEED_RECV status=0 action=call_transmit
  kworker/u128:2-33    [003]   155.723890: rpc_task_sleep:       task:000006cc@00000003 flags=ASYNC|MOVEABLE|NORTO|CRED_NOREF runstate=RUNNING|ACTIVE|NEED_XMIT|NEED_RECV status=-11 timeout=0 queue=xprt_sending
     kworker/1:2-115   [001]   155.733398: bprint:               do_tcp_sendpages: sk=0xffff88810a8f0a00 clearing SOCKWQ_ASYNC_NOSPACE
     kworker/1:2-115   [001]   155.733418: bprint:               do_tcp_sendpages: sk=0xffff88810a8f0a00 clearing SOCKWQ_ASYNC_NOSPACE
         openvpn-914   [001]   155.750263: bprint:               xs_write_space: sk=0xffff88810a8f0a00 SOCKWQ_ASYNC_NOSPACE was clear


>> For example, why check that flag rather than just waking
>> up xprt->sending unconditionally?
> 
> The socket code calls ->write_space() in all sorts of situations, so we
> need to distinguish between the cases where we are actually waiting for
> buffer memory, and the situations where we are not. Otherwise, we'd be
> calling xs_run_error_worker() all the time.

On my (admittedly limited) workloads, sk_stream_is_writeable()
does a good job of avoiding spurious wake-ups. However, to be
absolutely certain of our wake-up accounting, using a flag that
is local to the rpc_xprt and not overloaded might be wise?


--
Chuck Lever