Hello Daniel,. A few more comments: On Wed, Sep 1, 2021 at 5:51 AM Daniel P. Berrangé <berra...@redhat.com> wrote: > > On Tue, Aug 31, 2021 at 04:27:04PM -0400, Peter Xu wrote: > > On Tue, Aug 31, 2021 at 01:57:33PM +0100, Daniel P. Berrangé wrote: > > > On Tue, Aug 31, 2021 at 08:02:38AM -0300, Leonardo Bras wrote: > > > > MSG_ZEROCOPY is a feature that enables copy avoidance in TCP/UDP socket > > > > send calls. It does so by avoiding copying user data into kernel > > > > buffers. > > > > > > > > To make it work, three steps are needed: > > > > 1 - A setsockopt() system call, enabling SO_ZEROCOPY > > > > 2 - Passing down the MSG_ZEROCOPY flag for each send*() syscall > > > > 3 - Process the socket's error queue, dealing with any error > > > > > > AFAICT, this is missing the single most critical aspect of MSG_ZEROCOPY. > > > > > > It is non-obvious, but setting the MSG_ZEROCOPY flag turns sendmsg() > > > from a synchronous call to an asynchronous call. > > > > > > It is forbidden to overwrite/reuse/free the buffer passed to sendmsg > > > until an asynchronous completion notification has been received from > > > the socket error queue. These notifications are not required to > > > arrive in-order, even for a TCP stream, because the kernel hangs on > > > to the buffer if a re-transmit is needed. > > > > > > https://www.kernel.org/doc/html/v5.4/networking/msg_zerocopy.html > > > > > > "Page pinning also changes system call semantics. It temporarily > > > shares the buffer between process and network stack. Unlike with > > > copying, the process cannot immediately overwrite the buffer > > > after system call return without possibly modifying the data in > > > flight. Kernel integrity is not affected, but a buggy program > > > can possibly corrupt its own data stream." > > > > > > AFAICT, the design added in this patch does not provide any way > > > to honour these requirements around buffer lifetime. > > > > > > I can't see how we can introduce MSG_ZEROCOPY in any seemless > > > way. The buffer lifetime requirements imply need for an API > > > design that is fundamentally different for asynchronous usage, > > > with a callback to notify when the write has finished/failed. > > > > Regarding buffer reuse - it indeed has a very deep implication on the buffer > > being available and it's not obvious at all. Just to mention that the > > initial > > user of this work will make sure all zero copy buffers will be guest pages > > only > > (as it's only used in multi-fd), so they should always be there during the > > process. > > That is not the case when migration is using TLS, because the buffers > transmitted are the ciphertext buffer, not the plaintext guest page.
That makes sense. I am still working my way on Migration code, and I ended up not knowing that part. I think we can work on KTLS for this case, and implement something 'like zerocopy' for this. I added more details in the previous mail I sent you. > > > In short, we may just want to at least having a way to make sure all zero > > copied buffers are finished using and they're sent after some function > > returns > > (e.g., qio_channel_flush()). That may require us to do some accounting on > > when > > we called sendmsg(MSG_ZEROCOPY), meanwhile we should need to read out the > > ee_data field within SO_EE_ORIGIN_ZEROCOPY msg when we do recvmsg() for the > > error queue and keep those information somewhere too. > > > > Some other side notes that reached my mind.. > > > > The qio_channel_writev_full() may not be suitable for async operations, as > > the > > name "full" implies synchronous to me. So maybe we can add a new helper for > > zero copy on the channel? > > All the APIs that exist today are fundamentally only suitable for sync > operations. Supporting async correctly will definitely a brand new APIs > separate from what exists today. That's a great, I was thinking on implementing this as an optional method for QIOChannel, more details in the previous mail :). (sorry, I ended up reading/replying the mails in thread order) > > Regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| > Best regards, Leonardo Bras
