From: Fiona Ebner <f.ebner@proxmox.com>
To: "Proxmox VE development discussion" <pve-devel@lists.proxmox.com>,
"Fabian Grünbichler" <f.gruenbichler@proxmox.com>
Subject: Re: [pve-devel] [PATCH qemu-server v2 2/4] api: vm start: introduce nets-host-mtu parameter for migration compat
Date: Thu, 4 Sep 2025 11:28:16 +0200 [thread overview]
Message-ID: <4ceb8551-2a07-49fa-8810-4797a6597410@proxmox.com> (raw)
In-Reply-To: <1756975546.iilfeifhpw.astroid@yuna.none>
Am 04.09.25 um 11:11 AM schrieb Fabian Grünbichler:
> On September 3, 2025 4:22 pm, Fiona Ebner wrote:
>> diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
>> index b571e6c1..95db271b 100644
>> --- a/src/PVE/API2/Qemu.pm
>> +++ b/src/PVE/API2/Qemu.pm
>> @@ -3383,6 +3383,15 @@ __PACKAGE__->register_method({
>> default => 0,
>> description => 'Whether to migrate conntrack entries for running VMs.',
>> },
>> + 'nets-host-mtu' => {
>> + type => 'string',
>> + pattern => 'net\d+=\d+(,net\d+=\d+)*',
>> + optional => 1,
>> + description =>
>> + 'Used for migration compat. List of VirtIO network devices and their effective'
>> + . ' host_mtu setting according to the QEMU object model on the source side of'
>> + . ' the migration.',
>
> should we note here as well that `0` means "explicitly don't set host_mtu"?
Will add that.
>> @@ -1484,10 +1494,37 @@ sub print_netdevice_full {
>>
>> my $mtu = $net->{mtu};
>>
>> - if ($net->{model} eq 'virtio' && $net->{bridge}) {
>> + if (defined($host_mtu_migration)) {
>> + if ($host_mtu_migration) {
>> + if (defined($mtu) && $mtu != 1) {
>> + if ($mtu != $host_mtu_migration) {
>> + log_warn(
>> + "netdev $netid: using 'host_mtu=$host_mtu_migration' for migration compat,"
>> + . " but value different from value in configuration '$mtu'");
>> + } # else avoid being overly verbose if there is an explicit setting
>
> can this happen in practice (without manually editing the config or
> manual QMP commands)?
I don't think so. But since we have the info, I though it'd be good to
warn about it.
>
>> + } else {
>> + print "netdev $netid: using 'host_mtu=$host_mtu_migration' for migration compat\n";
>> + }
>> + } else {
>> + print "netdev $netid: not adding 'host_mtu' parameter for migration compat\n";
>> + }
>> + }
>
> this if here
>
>> +
>> + if (
>> + $net->{model} eq 'virtio'
>> + && $net->{bridge}
>> + && (!defined($host_mtu_migration) || $host_mtu_migration)
>> + ) {
>> my $bridge_mtu = PVE::Network::read_bridge_mtu($net->{bridge});
>>
>> - if (!defined($mtu) || $mtu == 1) {
>> + if ($host_mtu_migration) {
>
> and this if here could be combined?
How? You mean setting $mtu = $host_mtu_migration; early if there is a
non-zero value?
>
>> + $mtu = $host_mtu_migration;
>> + # TODO PVE 10 - upgrade to failure? Certain network traffic can break like
>> + # iperf3 -c 10.10.10.11 -u -l 2k when host_mtu=9000 and bridge MTU=1500
>> + if ($mtu > $bridge_mtu) {
>> + log_warn("netdev $netid: MTU '$mtu' is bigger than the bridge MTU '$bridge_mtu'");
>> + }
>> + } elsif (!defined($mtu) || $mtu == 1) {
>
> this could still be an if, then the newly added warning above can be
> dropped
>
>> $mtu = $bridge_mtu;
>> } elsif ($mtu < 576) {
>> die "netdev $netid: MTU '$mtu' is smaller than the IP minimum MTU '576'\n";
>> @@ -1495,7 +1532,7 @@ sub print_netdevice_full {
>> die "netdev $netid: MTU '$mtu' is bigger than the bridge MTU '$bridge_mtu'\n";
>
> since it is covered by this one here
If we want to go for early failure when host_mtu > bridge MTU then yes.
Because this is die, the above is warn ;) Doing that is fine by me, but
I wanted to propose the non-breaking option first. If we consider that
it's problematic in more cases than not, then I'll go for early failure.
And maybe I'll add a little more context to the error message (at least
when it happens for migration).
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
next prev parent reply other threads:[~2025-09-04 9:28 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-03 14:22 [pve-devel] [PATCH-SERIES qemu-server v2 0/4] virtio-net: fix migration between default/non-default MTUs, part one and two Fiona Ebner
2025-09-03 14:22 ` [pve-devel] [PATCH qemu-server v2 1/4] virtio-net: fix migration between default/non-default MTUs starting with machine version 10.0+pve1 Fiona Ebner
2025-09-03 14:22 ` [pve-devel] [PATCH qemu-server v2 2/4] api: vm start: introduce nets-host-mtu parameter for migration compat Fiona Ebner
2025-09-04 9:11 ` Fabian Grünbichler
2025-09-04 9:28 ` Fiona Ebner [this message]
2025-09-04 9:52 ` Fabian Grünbichler
2025-09-04 10:03 ` Fiona Ebner
2025-09-04 9:55 ` Fiona Ebner
2025-09-03 14:22 ` [pve-devel] [PATCH qemu-server v2 3/4] migration: preserve host_mtu for virtio-net devices Fiona Ebner
2025-09-03 14:22 ` [pve-devel] [PATCH qemu-server v2 stable-bookworm 4/4] " Fiona Ebner
2025-09-04 9:11 ` Fabian Grünbichler
2025-09-04 9:32 ` Fiona Ebner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4ceb8551-2a07-49fa-8810-4797a6597410@proxmox.com \
--to=f.ebner@proxmox.com \
--cc=f.gruenbichler@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox