From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nishanth Devarajan Subject: Re: [PATCH iproute2/net-next v3]tc: B.W limits can now be specified in %. Date: Sat, 25 Nov 2017 02:13:46 +0530 Message-ID: <20171124204345.GB13551@gmail.com> References: <20171117204335.GA17658@gmail.com> <20171124112528.13e44f11@xeon-e3> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, doucette@bu.edu, michel.machado@gmail.com To: Stephen Hemminger Return-path: Received: from mail-pg0-f66.google.com ([74.125.83.66]:34731 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753657AbdKXUoE (ORCPT ); Fri, 24 Nov 2017 15:44:04 -0500 Received: by mail-pg0-f66.google.com with SMTP id 4so16022348pge.1 for ; Fri, 24 Nov 2017 12:44:04 -0800 (PST) Content-Disposition: inline In-Reply-To: <20171124112528.13e44f11@xeon-e3> Sender: netdev-owner@vger.kernel.org List-ID: On Fri, Nov 24, 2017 at 11:25:28AM -0800, Stephen Hemminger wrote: > On Sat, 18 Nov 2017 02:13:38 +0530 > Nishanth Devarajan wrote: > > > This patch adapts the tc command line interface to allow bandwidth limits > > to be specified as a percentage of the interface's capacity. > > > > Adding this functionality requires passing the specified device string to > > each class/qdisc which changes the prototype for a couple of functions: the > > .parse_qopt and .parse_copt interfaces. The device string is a required > > parameter for tc-qdisc and tc-class, and when not specified, the kernel > > returns ENODEV. In this patch, if the user tries to specify a bandwidth > > percentage without naming the device, we return an error from userspace. > > > > v2: > > * Modified and moved int read_prop() from ip/iptuntap.c to lib/utils.c, > > to make it accessible to tc. > > > > v3: > > * Modified and moved int parse_percent() from tc/q_netem.c to ib/util.c for > > use in tc. > > > > * Changed couple variable names in int parse_percent_rate(). > > > > * Handled showing error message when device speed is unknown. > > > > * Updated man page to warn users that when specifying rates in %, tc only > > uses the current device speed and does not recalculate if it changes after. > > > > During cases when properties (like device speed) are unknown, read_prop() > > assumes that if the property file can be opened but not read, it means > > that the property is unknown. > > > > Signed-off by: Nishanth Devarajan > > > > Applied, but there were three things that I needed to change: > 1. The DCO tag is "Signed-off-by" not "Signed-off by" > 2. The revision history should be below the cut line --- in the mail message > so that it doesn't end up in the commit message. > 3. The qopt function declarations now are a really long line. > I will break them up. > Thanks for the help, and will do, I'll keep the feedback in mind for future patches, thanks. -Nishanth