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

[Hannes Reinecke]

On 4/18/23 20:21, Jakub Kicinski wrote:
> 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.
> 
Bear in mind that that distinction will become meaningless once dhowells 
patches for MSG_SPLICE are in, as that will get rid of sendpage() 
altogether.
(At least to my understanding.)

Cheers,

Hannes