[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v11 2/7] QIOChannelSocket: Implement io_writev zero copy flag
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v11 2/7] QIOChannelSocket: Implement io_writev zero copy flag & io_flush for CONFIG_LINUX |
Date: |
Thu, 5 May 2022 16:55:27 +0100 |
User-agent: |
Mutt/2.1.5 (2021-12-30) |
On Thu, May 05, 2022 at 12:42:47PM -0300, Leonardo Bras Soares Passos wrote:
>
> Hello Daniel,
>
> But what if this gets compiled in a Linux system without MSG_ZEROCOPY support?
> As qapi will have zero-copy-send as an option we could have this scenario:
>
> - User request migration using zero-copy-send
> - multifd_save_setup() will set write_flags = QIO_CHANNEL_WRITE_FLAG_ZERO_COPY
> - In qio_channel_socket_connect_sync(): setsockopt() part will be
> compiled-out, so no error here
> - above part in qio_channel_socket_writev() will be commented-out,
> which means write_flags will be ignored
> - sflags will not contain MSG_ZEROCOPY, so sendmsg() will use copy-mode
> - migration will succeed
>
> In the above case, the user has all the reason to think migration is
> using MSG_ZEROCOPY, but in fact it's quietly falling back to
> copy-mode.
I think we're ok because qio_channel_writev_full() does
if ((flags & QIO_CHANNEL_WRITE_FLAG_ZERO_COPY) &&
!qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_WRITE_ZERO_COPY)) {
error_setg_errno(errp, EINVAL,
"Requested Zero Copy feature is not available");
return -1;
}
and since there's no way for QIO_CHANNEL_FEATURE_WRITE_ZERO_COPY to
get set when MSG_ZEROCOPY is compiled out, we'll trigger the error
condition.
> That's why I suggested creating a 'global' config usiing SO_ZEROCOPY &
> MSG_ZEROCOPY & CONFIG_LINUX so we can use in qapi and have no chance
> of even offering zero-copy-send if we don't have it.
>
> Another local option is to do implement your suggestions, and also
> change qio_channel_socket_connect_sync() so it returns an error if
> MSG_ZEROCOPY && SO_ZEROCOPY is not present, such as:
>
> +#ifdef CONFIG_LINUX
> +#if defined(MSG_ZEROCOPY) && defined(SO_ZEROCOPY)
> + int ret, v = 1;
> + ret = setsockopt(fd, SOL_SOCKET, SO_ZEROCOPY, &v, sizeof(v));
> + if (ret == 0) {
> + /* Zero copy available on host */
> + qio_channel_set_feature(QIO_CHANNEL(ioc),
> + QIO_CHANNEL_FEATURE_WRITE_ZERO_COPY);
> + }
> +#else
> + error_setg_errno(errp, errno,"MSG_ZEROCOPY not available");
> + return -1;
> +#endif
> +#endif
Do we actually need the ifdef CONFIG_LINUX bit at all ?
Sufficient to just have the check for MSG_ZEROCOPY + SO_ZEROCOPY,
which will fail on non-Linux anyway.
With 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 :|
[PATCH v11 3/7] migration: Add zero-copy-send parameter for QMP/HMP for Linux, Leonardo Bras, 2022/05/04
[PATCH v11 4/7] migration: Add migrate_use_tls() helper, Leonardo Bras, 2022/05/04
[PATCH v11 5/7] multifd: multifd_send_sync_main now returns negative on error, Leonardo Bras, 2022/05/04
[PATCH v11 6/7] multifd: Send header packet without flags if zero-copy-send is enabled, Leonardo Bras, 2022/05/04
[PATCH v11 7/7] multifd: Implement zero copy write in multifd migration (multifd-zero-copy), Leonardo Bras, 2022/05/04