public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH v1 series 0/5] limit out and inrate of network device
@ 2020-09-11 10:08 Hannes Laimer
  2020-09-11 10:08 ` [pve-devel] [PATCH v1 pve-common 1/5] replace rate with out/in-rate in setup_tc_rate_limit and tap_rate_limit Hannes Laimer
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Hannes Laimer @ 2020-09-11 10:08 UTC (permalink / raw)
  To: pve-devel

'rate' is split into 'outrate' and 'inrate', so the upload and download
limit can be set individually. Old configs containing just 'rate' are
mapped to out/inrate. After opening the edit dialog in the gui 'rate'
will be removed from the config and out/inrate are added. In the WebUI
the field Rate is also replace with the two new fields.
Since we can only set rules for the tap-device in the host system, we
are only able to limit the downloadrate of the VM accuratelly(egress of
tap). Setting an uploadrate limit is neither precise nor consistent due
to the nature of ingress traffic shaping(ingress of tap/outrate of VM)

pve-common: Hannes Laimer (2):
  replace rate with out/in-rate in setup_tc_rate_limit and
    tap_rate_limit
  add out/in-rate parameter to tap_plug sub, keep version with just rate
    param

 src/PVE/Network.pm | 43 +++++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 16 deletions(-)


pve-network: Hannes Laimer (1):
  add out/in-rate parameter to tap_plug sub, keep version with just rate
    param

 PVE/Network/SDN/Zones.pm | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)


qemu-server: Hannes Laimer (1):
  add fields inrate and outrate to net_fmt, map rate of old configs to
    out/inrate

 PVE/QemuServer.pm             | 35 +++++++++++++++++++++++++++++------
 vm-network-scripts/pve-bridge |  9 ++++++---
 2 files changed, 35 insertions(+), 9 deletions(-)


pve-manager: Hannes Laimer (1):
  out/in-rate in network edit, keep rate to still be able to open old
    configs

 www/manager6/Parser.js           | 18 ++++++++++++++++++
 www/manager6/qemu/NetworkEdit.js | 27 ++++++++++++++++++++++++---
 2 files changed, 42 insertions(+), 3 deletions(-)

-- 
2.20.1





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

* [pve-devel] [PATCH v1 pve-common 1/5] replace rate with out/in-rate in setup_tc_rate_limit and tap_rate_limit
  2020-09-11 10:08 [pve-devel] [PATCH v1 series 0/5] limit out and inrate of network device Hannes Laimer
@ 2020-09-11 10:08 ` Hannes Laimer
  2021-02-06 14:29   ` Thomas Lamprecht
  2020-09-11 10:08 ` [pve-devel] [PATCH v1 pve-common 2/5] add out/in-rate parameter to tap_plug sub, keep version with just rate param Hannes Laimer
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Hannes Laimer @ 2020-09-11 10:08 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/PVE/Network.pm | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
index 12536c7..3e7a1c1 100644
--- a/src/PVE/Network.pm
+++ b/src/PVE/Network.pm
@@ -82,7 +82,7 @@ our $ipv4_mask_hash_localnet = {
 };
 
 sub setup_tc_rate_limit {
-    my ($iface, $rate, $burst) = @_;
+    my ($iface, $outrate, $inrate, $burst) = @_;
 
     # these are allowed / expected to fail, e.g. when there is no previous rate limit to remove
     eval { run_command("/sbin/tc class del dev $iface parent 1: classid 1:1 >/dev/null 2>&1"); };
@@ -90,29 +90,35 @@ sub setup_tc_rate_limit {
     eval { run_command("/sbin/tc qdisc del dev $iface ingress >/dev/null 2>&1"); };
     eval { run_command("/sbin/tc qdisc del dev $iface root >/dev/null 2>&1"); };
 
-    return if !$rate;
-
     # tbf does not work for unknown reason
     #$TC qdisc add dev $DEV root tbf rate $RATE latency 100ms burst $BURST
     # so we use htb instead
-    run_command("/sbin/tc qdisc add dev $iface root handle 1: htb default 1");
-    run_command("/sbin/tc class add dev $iface parent 1: classid 1:1 " .
-		"htb rate ${rate}bps burst ${burst}b");
-
-    run_command("/sbin/tc qdisc add dev $iface handle ffff: ingress");
-    run_command("/sbin/tc filter add dev $iface parent ffff: " .
-		"prio 50 basic " .
-		"police rate ${rate}bps burst ${burst}b mtu 64kb " .
-		"drop");
+
+    # inrate limits the egress of the tap device (=> inrate of the VM)
+    if($inrate) {
+	run_command("/sbin/tc qdisc add dev $iface root handle 1: htb default 1");
+	run_command("/sbin/tc class add dev $iface parent 1: classid 1:1 " .
+	    "htb rate ${inrate}bps burst ${burst}b");
+    }
+
+    # outrate limits the ingress of the tap device (=> outrate of the VM)
+    if($outrate) {
+	run_command("/sbin/tc qdisc add dev $iface handle ffff: ingress");
+	run_command("/sbin/tc filter add dev $iface parent ffff: " .
+	    "prio 50 basic " .
+	    "police rate ${outrate}bps burst ${burst}b mtu 64kb " .
+	    "drop")
+    }
 }
 
 sub tap_rate_limit {
-    my ($iface, $rate) = @_;
+    my ($iface, $outrate, $inrate) = @_;
 
-    $rate = int($rate*1024*1024) if $rate;
+    $outrate = int($outrate*1024*1024) if $outrate;
+    $inrate = int($inrate*1024*1024) if $inrate;
     my $burst = 1024*1024;
 
-    setup_tc_rate_limit($iface, $rate, $burst);
+    setup_tc_rate_limit($iface, $outrate, $inrate, $burst);
 }
 
 sub read_bridge_mtu {
-- 
2.20.1





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

* [pve-devel] [PATCH v1 pve-common 2/5] add out/in-rate parameter to tap_plug sub, keep version with just rate param
  2020-09-11 10:08 [pve-devel] [PATCH v1 series 0/5] limit out and inrate of network device Hannes Laimer
  2020-09-11 10:08 ` [pve-devel] [PATCH v1 pve-common 1/5] replace rate with out/in-rate in setup_tc_rate_limit and tap_rate_limit Hannes Laimer
@ 2020-09-11 10:08 ` Hannes Laimer
  2021-02-06 14:22   ` Thomas Lamprecht
  2021-04-29 14:57   ` Thomas Lamprecht
  2020-09-11 10:08 ` [pve-devel] [PATCH v1 pve-network 3/5] " Hannes Laimer
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 9+ messages in thread
From: Hannes Laimer @ 2020-09-11 10:08 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/PVE/Network.pm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
index 3e7a1c1..3b09cec 100644
--- a/src/PVE/Network.pm
+++ b/src/PVE/Network.pm
@@ -389,6 +389,11 @@ my $cleanup_firewall_bridge = sub {
 
 sub tap_plug {
     my ($iface, $bridge, $tag, $firewall, $trunks, $rate) = @_;
+    tap_plug($iface, $bridge, $tag, $firewall, $trunks, $rate, $rate);
+}
+
+sub tap_plug {
+    my ($iface, $bridge, $tag, $firewall, $trunks, $inrate, $outrate) = @_;
 
     #cleanup old port config from any openvswitch bridge
     eval {run_command("/usr/bin/ovs-vsctl del-port $iface", outfunc => sub {}, errfunc => sub {}) };
@@ -422,7 +427,7 @@ sub tap_plug {
 	}
     }
 
-    tap_rate_limit($iface, $rate);
+    tap_rate_limit($iface, $outrate, $inrate);
 }
 
 sub tap_unplug {
-- 
2.20.1





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

* [pve-devel] [PATCH v1 pve-network 3/5] add out/in-rate parameter to tap_plug sub, keep version with just rate param
  2020-09-11 10:08 [pve-devel] [PATCH v1 series 0/5] limit out and inrate of network device Hannes Laimer
  2020-09-11 10:08 ` [pve-devel] [PATCH v1 pve-common 1/5] replace rate with out/in-rate in setup_tc_rate_limit and tap_rate_limit Hannes Laimer
  2020-09-11 10:08 ` [pve-devel] [PATCH v1 pve-common 2/5] add out/in-rate parameter to tap_plug sub, keep version with just rate param Hannes Laimer
@ 2020-09-11 10:08 ` Hannes Laimer
  2020-09-11 10:08 ` [pve-devel] [PATCH v1 qemu-server 4/5] add fields inrate and outrate to net_fmt, map rate of old configs to out/inrate Hannes Laimer
  2020-09-11 10:08 ` [pve-devel] [PATCH v1 pve-manager 5/5] out/in-rate in network edit, keep rate to still be able to open old configs Hannes Laimer
  4 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2020-09-11 10:08 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 PVE/Network/SDN/Zones.pm | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/PVE/Network/SDN/Zones.pm b/PVE/Network/SDN/Zones.pm
index 143d6e5..0fe6098 100644
--- a/PVE/Network/SDN/Zones.pm
+++ b/PVE/Network/SDN/Zones.pm
@@ -278,10 +278,15 @@ sub veth_create {
 
 sub tap_plug {
     my ($iface, $bridge, $tag, $firewall, $trunks, $rate) = @_;
+    tap_plug($iface, $bridge, $tag, $firewall, $trunks, $rate, $rate);
+}
+
+sub tap_plug {
+    my ($iface, $bridge, $tag, $firewall, $trunks, $outrate, $inrate) = @_;
 
     my $vnet = PVE::Network::SDN::Vnets::get_vnet($bridge);
     if (!$vnet) { # fallback for classic bridge
-	PVE::Network::tap_plug($iface, $bridge, $tag, $firewall, $trunks, $rate);
+	PVE::Network::tap_plug($iface, $bridge, $tag, $firewall, $trunks, $outrate, $inrate);
 	return;
     }
 
@@ -292,7 +297,7 @@ sub tap_plug {
 	if $plugin_config->{nodes} && !defined($plugin_config->{nodes}->{$nodename});
 
     my $plugin = PVE::Network::SDN::Zones::Plugin->lookup($plugin_config->{type});
-    $plugin->tap_plug($plugin_config, $vnet, $tag, $iface, $bridge, $firewall, $trunks, $rate);
+    $plugin->tap_plug($plugin_config, $vnet, $tag, $iface, $bridge, $firewall, $trunks, $outrate, $inrate);
 }
 
 1;
-- 
2.20.1





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

* [pve-devel] [PATCH v1 qemu-server 4/5] add fields inrate and outrate to net_fmt, map rate of old configs to out/inrate
  2020-09-11 10:08 [pve-devel] [PATCH v1 series 0/5] limit out and inrate of network device Hannes Laimer
                   ` (2 preceding siblings ...)
  2020-09-11 10:08 ` [pve-devel] [PATCH v1 pve-network 3/5] " Hannes Laimer
@ 2020-09-11 10:08 ` Hannes Laimer
  2020-09-11 10:08 ` [pve-devel] [PATCH v1 pve-manager 5/5] out/in-rate in network edit, keep rate to still be able to open old configs Hannes Laimer
  4 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2020-09-11 10:08 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 PVE/QemuServer.pm             | 35 +++++++++++++++++++++++++++++------
 vm-network-scripts/pve-bridge |  9 ++++++---
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 2747c66..ff59d78 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -886,6 +886,18 @@ my $net_fmt = {
 	description => "Rate limit in mbps (megabytes per second) as floating point number.",
 	optional => 1,
     },
+    outrate => {
+	type => 'number',
+	minimum => 0,
+	description => "Upload rate limit in mbps (megabytes per second) as floating point number.",
+	optional => 1,
+    },
+    inrate => {
+	type => 'number',
+	minimum => 0,
+	description => "Download rate limit in mbps (megabytes per second) as floating point number.",
+	optional => 1,
+	},
     tag => {
 	type => 'integer',
 	minimum => 1, maximum => 4094,
@@ -1732,7 +1744,7 @@ sub parse_numa {
     return $res;
 }
 
-# netX: e1000=XX:XX:XX:XX:XX:XX,bridge=vmbr0,rate=<mbps>
+# netX: e1000=XX:XX:XX:XX:XX:XX,bridge=vmbr0,outrate=<mbps>,inrate=<mbps>,[rate=<mbps>]deprecated
 sub parse_net {
     my ($data) = @_;
 
@@ -3473,7 +3485,10 @@ sub config_to_command {
 	 next if !$d;
 
 	 $use_virtio = 1 if $d->{model} eq 'virtio';
-
+	# setting in/out-rate to rate if rate is set and in/out-rate is not set
+	# if in/out-rate was changed by the user, rate will always be empty
+	$d->{inrate} = $d->{rate} if !$d->{inrate} && $d->{rate};
+	$d->{outrate} = $d->{rate} if !$d->{outrate} && $d->{rate};
 	 if ($bootindex_hash->{n}) {
 	    $d->{bootindex} = $bootindex_hash->{n};
 	    $bootindex_hash->{n} += 1;
@@ -4625,6 +4640,12 @@ sub vmconfig_update_net {
 	    die "internal error" if $opt !~ m/net(\d+)/;
 	    my $iface = "tap${vmid}i$1";
 
+	    # setting out/in-rate to old rate if rate is set and both new and old out/in-rate are not set
+	    # needed for backwards-compatibility, only applies if old config has not changed since update
+            # if the out/in-rate was changed by the user rate will always be empty
+	    $newnet->{inrate} = $oldnet->{rate} if !$oldnet->{inrate} && !$newnet->{inrate} && $oldnet->{rate};
+	    $newnet->{outrate} = $oldnet->{rate} if !$oldnet->{outrate} && !$newnet->{outrate} && $oldnet->{rate};
+
 	    if (safe_string_ne($oldnet->{bridge}, $newnet->{bridge}) ||
 		safe_num_ne($oldnet->{tag}, $newnet->{tag}) ||
 		safe_string_ne($oldnet->{trunks}, $newnet->{trunks}) ||
@@ -4632,14 +4653,16 @@ sub vmconfig_update_net {
 		PVE::Network::tap_unplug($iface);
 
 		if ($have_sdn) {
-		    PVE::Network::SDN::Zones::tap_plug($iface, $newnet->{bridge}, $newnet->{tag}, $newnet->{firewall}, $newnet->{trunks}, $newnet->{rate});
+		    PVE::Network::SDN::Zones::tap_plug($iface, $newnet->{bridge}, $newnet->{tag}, $newnet->{firewall}, $newnet->{trunks}, $newnet->{outrate}, $newnet->{inrate});
 		} else {
-		    PVE::Network::tap_plug($iface, $newnet->{bridge}, $newnet->{tag}, $newnet->{firewall}, $newnet->{trunks}, $newnet->{rate});
+		    PVE::Network::tap_plug($iface, $newnet->{bridge}, $newnet->{tag}, $newnet->{firewall}, $newnet->{trunks}, $newnet->{outrate}, $newnet->{inrate});
 		}
-	    } elsif (safe_num_ne($oldnet->{rate}, $newnet->{rate})) {
+	    } elsif (safe_num_ne($oldnet->{inrate}, $newnet->{inrate}) ||
+		    safe_num_ne($oldnet->{outrate}, $newnet->{outrate})) {
+
 		# Rate can be applied on its own but any change above needs to
 		# include the rate in tap_plug since OVS resets everything.
-		PVE::Network::tap_rate_limit($iface, $newnet->{rate});
+		PVE::Network::tap_rate_limit($iface, $newnet->{outrate}, $newnet->{inrate});
 	    }
 
 	    if (safe_string_ne($oldnet->{link_down}, $newnet->{link_down})) {
diff --git a/vm-network-scripts/pve-bridge b/vm-network-scripts/pve-bridge
index d37ce33..4dfd369 100755
--- a/vm-network-scripts/pve-bridge
+++ b/vm-network-scripts/pve-bridge
@@ -42,13 +42,16 @@ die "unable to get network config '$netid'\n"
 
 my $net = PVE::QemuServer::parse_net($netconf);
 die "unable to parse network config '$netid'\n" if !$net;
-
+# setting out/in-rate to rate if rate is set and out/in-rate is not set
+# if out/int-rate was changed by the user, rate will always be empty
+$net->{outrate} = $net->{rate} if !$net->{outrate} && $net->{rate};
+$net->{inrate} = $net->{rate} if !$net->{inrate} && $net->{rate};
 if ($have_sdn) {
     PVE::Network::SDN::Zones::tap_create($iface, $net->{bridge});
-    PVE::Network::SDN::Zones::tap_plug($iface, $net->{bridge}, $net->{tag}, $net->{firewall}, $net->{trunks}, $net->{rate});
+    PVE::Network::SDN::Zones::tap_plug($iface, $net->{bridge}, $net->{tag}, $net->{firewall}, $net->{trunks}, $net->{outrate}, $net->{inrate});
 } else {
     PVE::Network::tap_create($iface, $net->{bridge});
-    PVE::Network::tap_plug($iface, $net->{bridge}, $net->{tag}, $net->{firewall}, $net->{trunks}, $net->{rate});
+    PVE::Network::tap_plug($iface, $net->{bridge}, $net->{tag}, $net->{firewall}, $net->{trunks}, $net->{outrate}, $net->{inrate});
 }
 
 exit 0;
-- 
2.20.1





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

* [pve-devel] [PATCH v1 pve-manager 5/5] out/in-rate in network edit, keep rate to still be able to open old configs
  2020-09-11 10:08 [pve-devel] [PATCH v1 series 0/5] limit out and inrate of network device Hannes Laimer
                   ` (3 preceding siblings ...)
  2020-09-11 10:08 ` [pve-devel] [PATCH v1 qemu-server 4/5] add fields inrate and outrate to net_fmt, map rate of old configs to out/inrate Hannes Laimer
@ 2020-09-11 10:08 ` Hannes Laimer
  4 siblings, 0 replies; 9+ messages in thread
From: Hannes Laimer @ 2020-09-11 10:08 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/manager6/Parser.js           | 18 ++++++++++++++++++
 www/manager6/qemu/NetworkEdit.js | 27 ++++++++++++++++++++++++---
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/www/manager6/Parser.js b/www/manager6/Parser.js
index b793a28e..eac31f2f 100644
--- a/www/manager6/Parser.js
+++ b/www/manager6/Parser.js
@@ -131,6 +131,10 @@ Ext.define('PVE.Parser', { statics: {
 		res.bridge = match_res[1];
 	    } else if ((match_res = p.match(/^rate=(\d+(\.\d+)?)$/)) !== null) {
 		res.rate = match_res[1];
+	    } else if ((match_res = p.match(/^outrate=(\d+(\.\d+)?)$/)) !== null) {
+		res.outrate = match_res[1];
+	    } else if ((match_res = p.match(/^inrate=(\d+(\.\d+)?)$/)) !== null) {
+		res.inrate = match_res[1];
 	    } else if ((match_res = p.match(/^tag=(\d+(\.\d+)?)$/)) !== null) {
 		res.tag = match_res[1];
 	    } else if ((match_res = p.match(/^firewall=(\d+)$/)) !== null) {
@@ -146,6 +150,14 @@ Ext.define('PVE.Parser', { statics: {
 		return false; // break
 	    }
 	});
+	if (res.rate) {
+	    if (!res.inrate) {
+		res.inrate = res.rate;
+	    }
+	    if (!res.outrate) {
+		res.outrate = res.rate;
+	    }
+	}
 
 	if (errors || !res.model) {
 	    return;
@@ -172,6 +184,12 @@ Ext.define('PVE.Parser', { statics: {
 	if (net.rate) {
 	    netstr += ",rate=" + net.rate;
 	}
+	if (net.outrate) {
+	    netstr += ",outrate=" + net.outrate;
+	}
+	if (net.inrate) {
+	    netstr += ",inrate=" + net.inrate;
+	}
 	if (net.queues) {
 	    netstr += ",queues=" + net.queues;
 	}
diff --git a/www/manager6/qemu/NetworkEdit.js b/www/manager6/qemu/NetworkEdit.js
index 3b093b46..8428a2aa 100644
--- a/www/manager6/qemu/NetworkEdit.js
+++ b/www/manager6/qemu/NetworkEdit.js
@@ -25,7 +25,16 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
 	} else {
 	    delete me.network.rate;
 	}
-
+	if (values.outrate) {
+	    me.network.outrate = values.outrate;
+	} else {
+	    delete me.network.outrate;
+	}
+	if (values.inrate) {
+	    me.network.inrate = values.inrate;
+	} else {
+	    delete me.network.inrate;
+	}
 	var params = {};
 
 	params[me.confid] = PVE.Parser.printQemuNetwork(me.network);
@@ -111,6 +120,8 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
 			    'model',
 			    'macaddr',
 			    'rate',
+			    'outrate',
+			    'inrate',
 			    'queues'
 			];
 			fields.forEach(function(fieldname) {
@@ -144,8 +155,18 @@ Ext.define('PVE.qemu.NetworkInputPanel', {
 	me.advancedColumn2 = [
 	    {
 		xtype: 'numberfield',
-		name: 'rate',
-		fieldLabel: gettext('Rate limit') + ' (MB/s)',
+		name: 'outrate',
+		fieldLabel: gettext('Upload limit') + ' (MB/s)',
+		minValue: 0,
+		maxValue: 10*1024,
+		value: '',
+		emptyText: 'unlimited',
+		allowBlank: true
+	    },
+		{
+		xtype: 'numberfield',
+		name: 'inrate',
+		fieldLabel: gettext('Download limit') + ' (MB/s)',
 		minValue: 0,
 		maxValue: 10*1024,
 		value: '',
-- 
2.20.1





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

* Re: [pve-devel] [PATCH v1 pve-common 2/5] add out/in-rate parameter to tap_plug sub, keep version with just rate param
  2020-09-11 10:08 ` [pve-devel] [PATCH v1 pve-common 2/5] add out/in-rate parameter to tap_plug sub, keep version with just rate param Hannes Laimer
@ 2021-02-06 14:22   ` Thomas Lamprecht
  2021-04-29 14:57   ` Thomas Lamprecht
  1 sibling, 0 replies; 9+ messages in thread
From: Thomas Lamprecht @ 2021-02-06 14:22 UTC (permalink / raw)
  To: Proxmox VE development discussion, Hannes Laimer

On 11.09.20 12:08, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  src/PVE/Network.pm | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
> index 3e7a1c1..3b09cec 100644
> --- a/src/PVE/Network.pm
> +++ b/src/PVE/Network.pm
> @@ -389,6 +389,11 @@ my $cleanup_firewall_bridge = sub {
>  
>  sub tap_plug {
>      my ($iface, $bridge, $tag, $firewall, $trunks, $rate) = @_;
> +    tap_plug($iface, $bridge, $tag, $firewall, $trunks, $rate, $rate);
> +}
> +
> +sub tap_plug {

why do you duplicate the tap_plug? that seems wrong!

> +    my ($iface, $bridge, $tag, $firewall, $trunks, $inrate, $outrate) = @_;
>  
>      #cleanup old port config from any openvswitch bridge
>      eval {run_command("/usr/bin/ovs-vsctl del-port $iface", outfunc => sub {}, errfunc => sub {}) };
> @@ -422,7 +427,7 @@ sub tap_plug {
>  	}
>      }
>  
> -    tap_rate_limit($iface, $rate);
> +    tap_rate_limit($iface, $outrate, $inrate);
>  }
>  
>  sub tap_unplug {
> 





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

* Re: [pve-devel] [PATCH v1 pve-common 1/5] replace rate with out/in-rate in setup_tc_rate_limit and tap_rate_limit
  2020-09-11 10:08 ` [pve-devel] [PATCH v1 pve-common 1/5] replace rate with out/in-rate in setup_tc_rate_limit and tap_rate_limit Hannes Laimer
@ 2021-02-06 14:29   ` Thomas Lamprecht
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Lamprecht @ 2021-02-06 14:29 UTC (permalink / raw)
  To: Proxmox VE development discussion, Hannes Laimer

On 11.09.20 12:08, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  src/PVE/Network.pm | 36 +++++++++++++++++++++---------------
>  1 file changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
> index 12536c7..3e7a1c1 100644
> --- a/src/PVE/Network.pm
> +++ b/src/PVE/Network.pm
> @@ -82,7 +82,7 @@ our $ipv4_mask_hash_localnet = {
>  };
>  
>  sub setup_tc_rate_limit {
> -    my ($iface, $rate, $burst) = @_;
> +    my ($iface, $outrate, $inrate, $burst) = @_;
>  
>      # these are allowed / expected to fail, e.g. when there is no previous rate limit to remove
>      eval { run_command("/sbin/tc class del dev $iface parent 1: classid 1:1 >/dev/null 2>&1"); };
> @@ -90,29 +90,35 @@ sub setup_tc_rate_limit {
>      eval { run_command("/sbin/tc qdisc del dev $iface ingress >/dev/null 2>&1"); };
>      eval { run_command("/sbin/tc qdisc del dev $iface root >/dev/null 2>&1"); };
>  
> -    return if !$rate;
> -
>      # tbf does not work for unknown reason
>      #$TC qdisc add dev $DEV root tbf rate $RATE latency 100ms burst $BURST
>      # so we use htb instead
> -    run_command("/sbin/tc qdisc add dev $iface root handle 1: htb default 1");
> -    run_command("/sbin/tc class add dev $iface parent 1: classid 1:1 " .
> -		"htb rate ${rate}bps burst ${burst}b");
> -
> -    run_command("/sbin/tc qdisc add dev $iface handle ffff: ingress");
> -    run_command("/sbin/tc filter add dev $iface parent ffff: " .
> -		"prio 50 basic " .
> -		"police rate ${rate}bps burst ${burst}b mtu 64kb " .
> -		"drop");
> +
> +    # inrate limits the egress of the tap device (=> inrate of the VM)
> +    if($inrate) {
> +	run_command("/sbin/tc qdisc add dev $iface root handle 1: htb default 1");
> +	run_command("/sbin/tc class add dev $iface parent 1: classid 1:1 " .
> +	    "htb rate ${inrate}bps burst ${burst}b");
> +    }

please transform run_command to array usage in a separate commit, that string
handling was always ugly and may break or result in command injection if it
gets called with a non-checked $iface (e.g., contains whitespaces or, well,
command injections).

> +
> +    # outrate limits the ingress of the tap device (=> outrate of the VM)
> +    if($outrate) {
> +	run_command("/sbin/tc qdisc add dev $iface handle ffff: ingress");
> +	run_command("/sbin/tc filter add dev $iface parent ffff: " .
> +	    "prio 50 basic " .
> +	    "police rate ${outrate}bps burst ${burst}b mtu 64kb " .
> +	    "drop")
> +    }
>  }
>  
>  sub tap_rate_limit {
> -    my ($iface, $rate) = @_;
> +    my ($iface, $outrate, $inrate) = @_;

please keep this backward compatible, i.e., if @_ only gets called with two
elements use it for both, $inrate and $outrate.

>  
> -    $rate = int($rate*1024*1024) if $rate;
> +    $outrate = int($outrate*1024*1024) if $outrate;
> +    $inrate = int($inrate*1024*1024) if $inrate;
>      my $burst = 1024*1024;
>  
> -    setup_tc_rate_limit($iface, $rate, $burst);
> +    setup_tc_rate_limit($iface, $outrate, $inrate, $burst);
>  }
>  
>  sub read_bridge_mtu {
> 





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

* Re: [pve-devel] [PATCH v1 pve-common 2/5] add out/in-rate parameter to tap_plug sub, keep version with just rate param
  2020-09-11 10:08 ` [pve-devel] [PATCH v1 pve-common 2/5] add out/in-rate parameter to tap_plug sub, keep version with just rate param Hannes Laimer
  2021-02-06 14:22   ` Thomas Lamprecht
@ 2021-04-29 14:57   ` Thomas Lamprecht
  1 sibling, 0 replies; 9+ messages in thread
From: Thomas Lamprecht @ 2021-04-29 14:57 UTC (permalink / raw)
  To: Proxmox VE development discussion, Hannes Laimer

On 11.09.20 12:08, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  src/PVE/Network.pm | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Network.pm b/src/PVE/Network.pm
> index 3e7a1c1..3b09cec 100644
> --- a/src/PVE/Network.pm
> +++ b/src/PVE/Network.pm
> @@ -389,6 +389,11 @@ my $cleanup_firewall_bridge = sub {
>  
>  sub tap_plug {
>      my ($iface, $bridge, $tag, $firewall, $trunks, $rate) = @_;
> +    tap_plug($iface, $bridge, $tag, $firewall, $trunks, $rate, $rate);
> +}
> +
> +sub tap_plug {
> +    my ($iface, $bridge, $tag, $firewall, $trunks, $inrate, $outrate) = @_;

FYI: This really cannot work, perl has no such overloading.

In theory you could pass the $in/out as array ref or even hash and check for that.
Something like the following (not tested):

my ($iface, $bridge, $tag, $firewall, $trunks, $rate) = @_;

my ($inrate, $outrate);

if (defined($rate) && ref($rate) eq 'HASH) {
    ($inrate, $outrate) = $rate->@{'in', 'out'};
} else {
    $inrate = $outrate = $rate;
}

>  
>      #cleanup old port config from any openvswitch bridge
>      eval {run_command("/usr/bin/ovs-vsctl del-port $iface", outfunc => sub {}, errfunc => sub {}) };
> @@ -422,7 +427,7 @@ sub tap_plug {
>  	}
>      }
>  
> -    tap_rate_limit($iface, $rate);
> +    tap_rate_limit($iface, $outrate, $inrate);
>  }
>  
>  sub tap_unplug {
> 





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

end of thread, other threads:[~2021-04-29 14:58 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-11 10:08 [pve-devel] [PATCH v1 series 0/5] limit out and inrate of network device Hannes Laimer
2020-09-11 10:08 ` [pve-devel] [PATCH v1 pve-common 1/5] replace rate with out/in-rate in setup_tc_rate_limit and tap_rate_limit Hannes Laimer
2021-02-06 14:29   ` Thomas Lamprecht
2020-09-11 10:08 ` [pve-devel] [PATCH v1 pve-common 2/5] add out/in-rate parameter to tap_plug sub, keep version with just rate param Hannes Laimer
2021-02-06 14:22   ` Thomas Lamprecht
2021-04-29 14:57   ` Thomas Lamprecht
2020-09-11 10:08 ` [pve-devel] [PATCH v1 pve-network 3/5] " Hannes Laimer
2020-09-11 10:08 ` [pve-devel] [PATCH v1 qemu-server 4/5] add fields inrate and outrate to net_fmt, map rate of old configs to out/inrate Hannes Laimer
2020-09-11 10:08 ` [pve-devel] [PATCH v1 pve-manager 5/5] out/in-rate in network edit, keep rate to still be able to open old configs Hannes Laimer

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