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
