Re: Re: [PATCH net] net/tls: sendfile fails with ktls offload

[rohit maheshwari <rohitm@chelsio.com>]

>> > -----Original Message-----
>> > From: Jakub Kicinski <kuba@kernel.org>
>> > Sent: Friday, September 25, 2020 3:27 AM
>> > To: Rohit Maheshwari <rohitm@chelsio.com>
>> > Cc: netdev@vger.kernel.org; davem@davemloft.net; 
>> vakul.garg@nxp.com; secdev <secdev@chelsio.com>
>> > Subject: Re: [PATCH net] net/tls: sendfile fails with ktls offload
>> >
>
>> > Also shouldn't we update this field or destroy the record before 
>> the break on line 478? If more is set, and payload is lesser than the 
>> max size, then we need to
>> hold on to get next sendpage and continue adding frags in the same 
>> record.
>> So I don't think we need to do any update or destroy the record. Please
>> correct me if I am wrong here.
>
> Agreed, if more is set we should continue appending.
>
> What I'm saying is that we may exit the loop on line 478 or 525 without
> updating pending_open_record_frags. So if pending_open_record_frags is
> set, we'd be in a position where there is no data in the record, yet
> pending_open_record_frags is set. Won't subsequent cmsg send not cause 
> a zero length record to be generated?
> Exit on line 478 can get triggered if sk_page_frag_refill() fails, and 
> then by
Exit on line 478 can get triggered if sk_page_frag_refill() fails,
and then by exiting, it will hit line 529 and will return 'rc =
orig_size - size', so I am sure we don't need to do anything else
there. Exit on line 525 will be, due to do_tcp_sendpage(), and I
think pending_open_record_frags won't be set if this is the last
record. And if it is not the last record, do_tcp_sendpage will be
failing for a complete and correct record, that doesn't need to be
destroyed and at this very moment pending_open_record_frags
will suggest that there is more data (unrelated to current failing
record), which actually is correct. I think, there won't be cmsg if
pending_open_record_frags is set.