Re: [PATCH 08/18] nvme-tcp: do not set MSG_SENDPAGE_NOTLAST

Jakub Kicinski
2023-04-18T18:21:40Z

On Tue, 18 Apr 2023 12:33:03 +0200 Hannes Reinecke wrote:
> Ah-ha. Found it.
>
> Turns out to be an arguably invalid control flow on our side.
>
> The initial 'connect' command is a command PDU with inline data.
> So in our (current) flow we do a 'sendpage()' with the
> MORE/SENDPAGE_NOTLAST flag set as we have to transfer more data.
> Ok so far.
> But for transmitting the actual data we evaluate 'sendpage_ok()', which
> returns _false_. And then we continue to call 'sendmsg()' for the inline
> data.
> This confuses the TLS software stack as sendpage() and sendmsg() are two
> distinct code paths, and we end up with a TX stall.
>
> Solution: check 'sendpage_ok()' when we have inline data and use
> 'sendmsg' for the command pdu if required.
>
> It always pays to dig deeper if one's not exactly sure what's happening ...
>
> Will be updating the patchset.

Hm, yeah, sendpage(NOT_LAST)+sendmsg() could indeed be buggy/untested.
It wasn't something we could have tested from user space, we'd need
kunit or some such :(

Adding it the backlog. And adding Vadim to CC in case he has cycles
to take a look.