public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH v4 manager] ui: vm network: allow to override MTU for virtio devices
@ 2022-05-16  9:37 Oguz Bektas
  2022-05-18 13:13 ` Dominik Csapak
  0 siblings, 1 reply; 2+ messages in thread
From: Oguz Bektas @ 2022-05-16  9:37 UTC (permalink / raw)
  To: pve-devel

we already have the 'mtu' option in the API, so we can just expose
that option inside the 'Advanced' menu for virtio network interfaces.

Reviewed-By: Aaron Lauterer <a.lauterer@proxmox.com>
Tested-By: Aaron Lauterer <a.lauterer@proxmox.com>
Tested-By: Dylan Whyte <d.whyte@proxmox.com>
Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---

v3->v4:
* show tooltip message about inheritance if mtu is 1


 www/manager6/qemu/NetworkEdit.js | 34 ++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/www/manager6/qemu/NetworkEdit.js b/www/manager6/qemu/NetworkEdit.js
index b39cffdc..6e56b73d 100644
--- a/www/manager6/qemu/NetworkEdit.js
+++ b/www/manager6/qemu/NetworkEdit.js
@@ -19,6 +19,7 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
 	me.network.macaddr = values.macaddr;
 	me.network.disconnect = values.disconnect;
 	me.network.queues = values.queues;
+	me.network.mtu = values.mtu;
 
 	if (values.rate) {
 	    me.network.rate = values.rate;
@@ -33,6 +34,17 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
 	return params;
     },
 
+    viewModel: {
+	data: {
+	    network_model: '',
+	    mtu: 1500,
+	},
+	formulas: {
+	    isVirtio: (get) => get('network_model') === 'virtio',
+	    showMTUHint: (get) => get('mtu') === 1,
+	},
+    },
+
     setNetwork: function(confid, data) {
 	var me = this;
 
@@ -112,6 +124,7 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
 			    'macaddr',
 			    'rate',
 			    'queues',
+			    'mtu',
 			];
 			fields.forEach(function(fieldname) {
 			    me.down('field[name='+fieldname+']').setDisabled(value);
@@ -130,6 +143,7 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
 		xtype: 'pveNetworkCardSelector',
 		name: 'model',
 		fieldLabel: gettext('Model'),
+		bind: '{network_model}',
 		value: PVE.qemu.OSDefaults.generic.networkCard,
 		allowBlank: false,
 	    },
@@ -161,6 +175,26 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
 		value: '',
 		allowBlank: true,
 	    },
+	    {
+		xtype: 'proxmoxintegerfield',
+		name: 'mtu',
+		fieldLabel: 'MTU',
+		bind: {
+		    disabled: '{!isVirtio}',
+		    value: '{mtu}',
+		},
+		minValue: 1,
+		maxValue: 65520,
+		allowBlank: true,
+	    },
+	    {
+		xtype: 'displayfield',
+		userCls: 'pmx-hint',
+		value: gettext("mtu=1 is a special value, the MTU value will be inherited from the current bridge"),
+		bind: {
+		    hidden: '{!showMTUHint}',
+		},
+	    },
 	];
 
 	me.callParent();
-- 
2.30.2





^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [pve-devel] [PATCH v4 manager] ui: vm network: allow to override MTU for virtio devices
  2022-05-16  9:37 [pve-devel] [PATCH v4 manager] ui: vm network: allow to override MTU for virtio devices Oguz Bektas
@ 2022-05-18 13:13 ` Dominik Csapak
  0 siblings, 0 replies; 2+ messages in thread
From: Dominik Csapak @ 2022-05-18 13:13 UTC (permalink / raw)
  To: Proxmox VE development discussion, Oguz Bektas

ok so i noticed the following:

when i have a virtio-nic with no mtu set, we populate the field with
'1500' (even when we don't show the advanced fields) and pressing
ok, changes the mtu (to 1500)

what is the default for that value? none?
in any case, the default should be the emptyText, and we
should not set the field to a value when it's not set
in the config

i guess this would work if you set the 'mtu' to undefined/null/empty string in
the viewmodel, and set the emptyText appropriately

otherwise LGTM

On 5/16/22 11:37, Oguz Bektas wrote:
> we already have the 'mtu' option in the API, so we can just expose
> that option inside the 'Advanced' menu for virtio network interfaces.
> 
> Reviewed-By: Aaron Lauterer <a.lauterer@proxmox.com>
> Tested-By: Aaron Lauterer <a.lauterer@proxmox.com>
> Tested-By: Dylan Whyte <d.whyte@proxmox.com>
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
> 
> v3->v4:
> * show tooltip message about inheritance if mtu is 1
> 
> 
>   www/manager6/qemu/NetworkEdit.js | 34 ++++++++++++++++++++++++++++++++
>   1 file changed, 34 insertions(+)
> 
> diff --git a/www/manager6/qemu/NetworkEdit.js b/www/manager6/qemu/NetworkEdit.js
> index b39cffdc..6e56b73d 100644
> --- a/www/manager6/qemu/NetworkEdit.js
> +++ b/www/manager6/qemu/NetworkEdit.js
> @@ -19,6 +19,7 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>   	me.network.macaddr = values.macaddr;
>   	me.network.disconnect = values.disconnect;
>   	me.network.queues = values.queues;
> +	me.network.mtu = values.mtu;
>   
>   	if (values.rate) {
>   	    me.network.rate = values.rate;
> @@ -33,6 +34,17 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>   	return params;
>       },
>   
> +    viewModel: {
> +	data: {
> +	    network_model: '',
> +	    mtu: 1500,
> +	},
> +	formulas: {
> +	    isVirtio: (get) => get('network_model') === 'virtio',
> +	    showMTUHint: (get) => get('mtu') === 1,
> +	},
> +    },
> +
>       setNetwork: function(confid, data) {
>   	var me = this;
>   
> @@ -112,6 +124,7 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>   			    'macaddr',
>   			    'rate',
>   			    'queues',
> +			    'mtu',
>   			];
>   			fields.forEach(function(fieldname) {
>   			    me.down('field[name='+fieldname+']').setDisabled(value);
> @@ -130,6 +143,7 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>   		xtype: 'pveNetworkCardSelector',
>   		name: 'model',
>   		fieldLabel: gettext('Model'),
> +		bind: '{network_model}',
>   		value: PVE.qemu.OSDefaults.generic.networkCard,
>   		allowBlank: false,
>   	    },
> @@ -161,6 +175,26 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
>   		value: '',
>   		allowBlank: true,
>   	    },
> +	    {
> +		xtype: 'proxmoxintegerfield',
> +		name: 'mtu',
> +		fieldLabel: 'MTU',
> +		bind: {
> +		    disabled: '{!isVirtio}',
> +		    value: '{mtu}',
> +		},
> +		minValue: 1,
> +		maxValue: 65520,
> +		allowBlank: true,
> +	    },
> +	    {
> +		xtype: 'displayfield',
> +		userCls: 'pmx-hint',
> +		value: gettext("mtu=1 is a special value, the MTU value will be inherited from the current bridge"),
> +		bind: {
> +		    hidden: '{!showMTUHint}',
> +		},
> +	    },
>   	];
>   
>   	me.callParent();





^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-05-18 13:13 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-16  9:37 [pve-devel] [PATCH v4 manager] ui: vm network: allow to override MTU for virtio devices Oguz Bektas
2022-05-18 13:13 ` Dominik Csapak

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