From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 8E38360A95 for ; Sat, 6 Feb 2021 15:29:14 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 86156121CE for ; Sat, 6 Feb 2021 15:29:14 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id A52FA121C0 for ; Sat, 6 Feb 2021 15:29:10 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 667F741B6F for ; Sat, 6 Feb 2021 15:29:10 +0100 (CET) Message-ID: <58efd38e-a7c8-d60e-a3be-9ac0e5cc198a@proxmox.com> Date: Sat, 6 Feb 2021 15:29:09 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:86.0) Gecko/20100101 Thunderbird/86.0 Content-Language: en-US To: Proxmox VE development discussion , Hannes Laimer References: <20200911100816.80543-1-h.laimer@proxmox.com> <20200911100816.80543-2-h.laimer@proxmox.com> From: Thomas Lamprecht In-Reply-To: <20200911100816.80543-2-h.laimer@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.009 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.105 Looks like a legit reply (A) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH v1 pve-common 1/5] replace rate with out/in-rate in setup_tc_rate_limit and tap_rate_limit X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 06 Feb 2021 14:29:14 -0000 On 11.09.20 12:08, Hannes Laimer wrote: > Signed-off-by: Hannes Laimer > --- > 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 { >