On Wed, 20 May 2020 01:49:33 +0300 Vadim Fedorenko wrote: > On 20.05.2020 01:04, Jakub Kicinski wrote: > > On Tue, 19 May 2020 13:20:43 +0300 Vadim Fedorenko wrote: > >> bpf_exec_tx_verdict() can return negative value for copied > >> variable. In that case this value will be pushed back to caller > >> and the real error code will be lost. Fix it using signed type and > >> checking for positive value. > >> > >> Fixes: d10523d0b3d7 ("net/tls: free the record on encryption error") > >> Fixes: d3b18ad31f93 ("tls: add bpf support to sk_msg handling") > >> Signed-off-by: Vadim Fedorenko <vfedore...@novek.ru> > > If the error encountered is transient we will still drop some data from > > the stream, because the record that was freed may have included data > > from a previous send call. Still, cleaning up the error code seems like > > an improvement. > > > > John, do you have an opinion on this? > > Jakub, maybe it is better to free only in case of fatal encryption error? I > mean > when sk->sk_err is EBADMSG. Because in case of ENOMEM we will iterate to > alloc_payload and in case of ENOSPC we will return good return code and send > open_rec again on next call. The EBADMSG state is the only fatal state that > needs > freeing of allocated record.
Sounds good!