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

Sagi Grimberg
2023-04-17T15:28:15Z

>> nak from me.
>>
>>> MSG_SENDPAGE_NOTLAST was introduced with commit 35f9c09fe9c7
>>> ("tcp: tcp_sendpages() should call tcp_push() once") to fix issues
>>> with the ->sendfile() call, and the implication seems to be that
>>> the flag should be strictly internal to sendfile.
>>
>> no, the use case was to allow sendpage consumers to build larger
>> tso packets for large payloads (splice being the primary one), nvme-tcp
>> also benefits from it (as do others), just like sendfile.
>>
>>> Hence we shouldn't be setting it on nvme-tcp, and anyway doing
>>> so will cause TLS sending invalid data.
>>
>> That is something else. I don't recall the full details on the
>> discussion with Jakub, but it was not my intention to strike
>> it altogether, just remove it if !sendpage_ok.
>>
> But then we're running afoul with tls_sw_do_sendpage(), which uses
> MSG_SENDPAGE_NOTLAST as a record indicator. Forwarding
> MSG_SENDPAGE_NOTLAST to tls results in incorrect TLS records on the wire
> and a transmission stall.
>
> Took me days to figure that out.
>
> If you have a better idea, I'm all ears.

So sendfile is unsupported over kTLS? Or is there some special
case for it in the upper layer?

Jakub?