public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
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

  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
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal