From patchwork Wed Feb 17 00:56:23 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathieu Desnoyers X-Patchwork-Id: 583797 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 9A162140213 for ; Wed, 17 Feb 2016 11:57:22 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933607AbcBQA5D (ORCPT ); Tue, 16 Feb 2016 19:57:03 -0500 Received: from mail.efficios.com ([78.47.125.74]:47655 "EHLO mail.efficios.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933415AbcBQA5B (ORCPT ); Tue, 16 Feb 2016 19:57:01 -0500 Received: from localhost (localhost.localdomain [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id 83473340667; Wed, 17 Feb 2016 00:56:59 +0000 (UTC) Received: from mail.efficios.com ([127.0.0.1]) by localhost (evm-mail-1.efficios.com [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id 8iDDIl9s6b43; Wed, 17 Feb 2016 00:56:54 +0000 (UTC) Received: from localhost (localhost.localdomain [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id CA218340674; Wed, 17 Feb 2016 00:56:54 +0000 (UTC) X-Virus-Scanned: amavisd-new at efficios.com Received: from mail.efficios.com ([127.0.0.1]) by localhost (evm-mail-1.efficios.com [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id uH-iaHu36TwP; Wed, 17 Feb 2016 00:56:54 +0000 (UTC) Received: from thinkos.internal.efficios.com (cable-192.222.213.99.electronicbox.net [192.222.213.99]) by mail.efficios.com (Postfix) with ESMTPSA id ABD27340667; Wed, 17 Feb 2016 00:56:53 +0000 (UTC) From: Mathieu Desnoyers To: "David S . Miller" Cc: linux-kernel@vger.kernel.org, Mathieu Desnoyers , Phil Sutter , Jamal Hadi Salim , netdev@vger.kernel.org, stable@vger.kernel.org Subject: [PATCH] Revert "net: sched: drop all special handling of tx_queue_len == 0" Date: Tue, 16 Feb 2016 19:56:23 -0500 Message-Id: <1455670592-5649-1-git-send-email-mathieu.desnoyers@efficios.com> X-Mailer: git-send-email 2.1.4 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org This reverts commit 348e3435cbefa815bd56a5205c1412b5afe7b92e. It breaks HTB classful qdiscs on the loopback interface. It has been broken since kernel v4.2. The offending commit has been identified by bissection of the issue with the following test-case. It appears that the loopback interface does indeed still have tx_queue_len == 0. Reverting the commit on a v4.4.1 kernel fixes the issue. When the problem occurs, no network traffic (at all), not even an ICMP ping, can go through the loopback interface. The following bash script reproduces the issue: SESSIOND_CTRL_PORT=5342 SESSIOND_DATA_PORT=5343 DEFAULT_IF="lo" function set_bw_limit { limit=$1 ctrlportlimit=$(($limit/10)) [ $ctrlportlimit = 0 ] && ctrlportlimit=1 dataportlimit=$((9*${ctrlportlimit})) tc qdisc add dev $DEFAULT_IF root handle 1: htb default 15 tc class add dev $DEFAULT_IF parent 1: classid 1:1 htb rate ${limit}kbit ceil ${limit}kbit tc class add dev $DEFAULT_IF parent 1:1 classid 1:10 htb rate ${ctrlportlimit}kbit ceil ${limit}kbit prio 1 tc class add dev $DEFAULT_IF parent 1:1 classid 1:11 htb rate ${dataportlimit}kbit ceil ${limit}kbit prio 2 tc filter add dev $DEFAULT_IF parent 1: protocol ip u32 match ip dport $SESSIOND_CTRL_PORT 0xffff flowid 1:10 tc filter add dev $DEFAULT_IF parent 1: protocol ip u32 match ip dport $SESSIOND_DATA_PORT 0xffff flowid 1:11 echo "Set bandwidth limits to ${limit}kbits, ${ctrlportlimit} for control and ${dataportlimit} for data" } function reset_bw_limit { tc qdisc del dev $DEFAULT_IF root echo "Reset bandwith limits" } trap reset_bw_limit SIGINT SIGTERM set_bw_limit 3200 sleep 1 ping localhost Signed-off-by: Mathieu Desnoyers Reported-by: Jonathan Rajotte-Julien CC: Phil Sutter CC: Jamal Hadi Salim CC: David S. Miller CC: netdev@vger.kernel.org CC: stable@vger.kernel.org # 4.2+ --- net/sched/sch_fifo.c | 2 +- net/sched/sch_gred.c | 8 +++++--- net/sched/sch_htb.c | 6 ++++-- net/sched/sch_plug.c | 8 ++++++-- net/sched/sch_sfb.c | 2 +- 5 files changed, 17 insertions(+), 9 deletions(-) diff --git a/net/sched/sch_fifo.c b/net/sched/sch_fifo.c index 2177eac..2e2398c 100644 --- a/net/sched/sch_fifo.c +++ b/net/sched/sch_fifo.c @@ -54,7 +54,7 @@ static int fifo_init(struct Qdisc *sch, struct nlattr *opt) bool is_bfifo = sch->ops == &bfifo_qdisc_ops; if (opt == NULL) { - u32 limit = qdisc_dev(sch)->tx_queue_len; + u32 limit = qdisc_dev(sch)->tx_queue_len ? : 1; if (is_bfifo) limit *= psched_mtu(qdisc_dev(sch)); diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c index 8010510..abb9f2f 100644 --- a/net/sched/sch_gred.c +++ b/net/sched/sch_gred.c @@ -512,9 +512,11 @@ static int gred_init(struct Qdisc *sch, struct nlattr *opt) if (tb[TCA_GRED_LIMIT]) sch->limit = nla_get_u32(tb[TCA_GRED_LIMIT]); - else - sch->limit = qdisc_dev(sch)->tx_queue_len - * psched_mtu(qdisc_dev(sch)); + else { + u32 qlen = qdisc_dev(sch)->tx_queue_len ? : 1; + + sch->limit = qlen * psched_mtu(qdisc_dev(sch)); + } return gred_change_table_def(sch, tb[TCA_GRED_DPS]); } diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c index 15ccd7f..366ba83 100644 --- a/net/sched/sch_htb.c +++ b/net/sched/sch_htb.c @@ -1048,9 +1048,11 @@ static int htb_init(struct Qdisc *sch, struct nlattr *opt) if (tb[TCA_HTB_DIRECT_QLEN]) q->direct_qlen = nla_get_u32(tb[TCA_HTB_DIRECT_QLEN]); - else + else { q->direct_qlen = qdisc_dev(sch)->tx_queue_len; - + if (q->direct_qlen < 2) /* some devices have zero tx_queue_len */ + q->direct_qlen = 2; + } if ((q->rate2quantum = gopt->rate2quantum) < 1) q->rate2quantum = 1; q->defcls = gopt->defcls; diff --git a/net/sched/sch_plug.c b/net/sched/sch_plug.c index 5abfe44..ade9445 100644 --- a/net/sched/sch_plug.c +++ b/net/sched/sch_plug.c @@ -130,8 +130,12 @@ static int plug_init(struct Qdisc *sch, struct nlattr *opt) q->unplug_indefinite = false; if (opt == NULL) { - q->limit = qdisc_dev(sch)->tx_queue_len - * psched_mtu(qdisc_dev(sch)); + /* We will set a default limit of 100 pkts (~150kB) + * in case tx_queue_len is not available. The + * default value is completely arbitrary. + */ + u32 pkt_limit = qdisc_dev(sch)->tx_queue_len ? : 100; + q->limit = pkt_limit * psched_mtu(qdisc_dev(sch)); } else { struct tc_plug_qopt *ctl = nla_data(opt); diff --git a/net/sched/sch_sfb.c b/net/sched/sch_sfb.c index 5bbb633..d5c1c1d 100644 --- a/net/sched/sch_sfb.c +++ b/net/sched/sch_sfb.c @@ -502,7 +502,7 @@ static int sfb_change(struct Qdisc *sch, struct nlattr *opt) limit = ctl->limit; if (limit == 0) - limit = qdisc_dev(sch)->tx_queue_len; + limit = max_t(u32, qdisc_dev(sch)->tx_queue_len, 1); child = fifo_create_dflt(sch, &pfifo_qdisc_ops, limit); if (IS_ERR(child))