Re: [RFC PATCH v2 1/8] crypto: Allow gracefully ending the TLS session

[Peter Xu <peterx@redhat.com>]

On Fri, Feb 07, 2025 at 02:55:57PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Fri, Feb 07, 2025 at 11:27:51AM -0300, Fabiano Rosas wrote:
> >> QEMU's TLS session code provides no way to call gnutls_bye() to
> >> terminate a TLS session. Callers of qcrypto_tls_session_read() can
> >> choose to ignore a GNUTLS_E_PREMATURE_TERMINATION error by setting the
> >> gracefulTermination argument.
> >> 
> >> The QIOChannelTLS ignores the premature termination error whenever
> >> shutdown() has already been issued. This is not enough anymore for the
> >> migration code due to changes [1] in the synchronization between
> >> migration source and destination.
> >
> > This sentence seems to say commit [1] changed something on the tls
> > condition, but IMHO fundamentally the issue is multifd recv thread model
> > that relies on blocking readv() rather than request-based (like what src
> > multifd does).
> >
> > Now src uses either shutdown() or close() to kick dest multifd recv threads
> > out from readv().  That has nothing to do with what we do during complete()
> > with those sync messages.. referencing it is ok, but we'll need to
> > reference also the other commit to be clear pre-9.0 can also be prone to
> > this.  To me, it's more important to mention the root cause on the multifd
> > recv thread model, which requires explicit tls terminations.
> >
> 
> I didn't want to go into too much detail in a commit for crypto/. The

You already did so by referencing a multifd commit that changes how
complete() works!

> motivation for *this* patch is just: migration needs it. What about:
> 
>  The QIOChannelTLS ignores the premature termination error whenever
>  shutdown() has already been issued. This was found to be not enough for
>  the migration code because shutdown() might not have been issued before
>  the connection is terminated.

Looks good to me, thanks.

-- 
Peter Xu