[{"id":1757354,"web_url":"http://patchwork.ozlabs.org/comment/1757354/","msgid":"<1503665377.18816.2.camel@edumazet-glaptop3.roam.corp.google.com>","list_archive_url":null,"date":"2017-08-25T12:49:37","subject":"Re: [PATCH net-next RESEND] sched: sfq: drop packets after root\n\tqdisc lock is released","submitter":{"id":20106,"url":"http://patchwork.ozlabs.org/api/people/20106/","name":"Eric Dumazet","email":"erdnetdev@gmail.com"},"content":"On Fri, 2017-08-25 at 15:43 +0800, gfree.wind@vip.163.com wrote:\n> From: Gao Feng <gfree.wind@vip.163.com>\n> \n> The commit 520ac30f4551 (\"net_sched: drop packets after root qdisc lock\n> is released) made a big change of tc for performance. But there are\n> some points which are not changed in SFQ enqueue operation.\n> 1. Fail to find the SFQ hash slot;\n> 2. When the queue is full;\n> \n> Now use qdisc_drop instead free skb directly.\n\n\nThanks for doing this work.\n\nI have one comment\n\n> \n> Signed-off-by: Gao Feng <gfree.wind@vip.163.com>\n> ---\n>  net/sched/sch_sfq.c | 16 ++++++++++------\n>  1 file changed, 10 insertions(+), 6 deletions(-)\n> \n> diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c\n> index 82469ef..8841f4d 100644\n> --- a/net/sched/sch_sfq.c\n> +++ b/net/sched/sch_sfq.c\n> @@ -292,7 +292,7 @@ static inline void slot_queue_add(struct sfq_slot *slot, struct sk_buff *skb)\n>  \tslot->skblist_prev = skb;\n>  }\n>  \n> -static unsigned int sfq_drop(struct Qdisc *sch)\n> +static unsigned int sfq_drop(struct Qdisc *sch, struct sk_buff **to_free)\n>  {\n>  \tstruct sfq_sched_data *q = qdisc_priv(sch);\n>  \tsfq_index x, d = q->cur_depth;\n> @@ -310,9 +310,13 @@ static unsigned int sfq_drop(struct Qdisc *sch)\n>  \t\tslot->backlog -= len;\n>  \t\tsfq_dec(q, x);\n>  \t\tsch->q.qlen--;\n> -\t\tqdisc_qstats_drop(sch);\n>  \t\tqdisc_qstats_backlog_dec(sch, skb);\n> -\t\tkfree_skb(skb);\n> +\t\tif (likely(to_free)) {\n> +\t\t\tqdisc_drop(skb, sch, to_free);\n> +\t\t} else {\n> +\t\t\tqdisc_qstats_drop(sch);\n> +\t\t\tkfree_skb(skb);\n\n                         rtnl_kfree_skbs(skb, skb);  ?\n\nOr even better provide a non NULL to_free from sfq_change()\n\nThen call rtnl_kfree_skbs() once from sfq_change()\n\n\n> +\t\t}\n>  \t\treturn len;\n>  \t}\n>  \n> @@ -360,7 +364,7 @@ static int sfq_headdrop(const struct sfq_sched_data *q)\n>  \tif (hash == 0) {\n>  \t\tif (ret & __NET_XMIT_BYPASS)\n>  \t\t\tqdisc_qstats_drop(sch);\n> -\t\tkfree_skb(skb);\n> +\t\t__qdisc_drop(skb, to_free);\n>  \t\treturn ret;\n>  \t}\n>  \thash--;\n> @@ -465,7 +469,7 @@ static int sfq_headdrop(const struct sfq_sched_data *q)\n>  \t\treturn NET_XMIT_SUCCESS;\n>  \n>  \tqlen = slot->qlen;\n> -\tdropped = sfq_drop(sch);\n> +\tdropped = sfq_drop(sch, to_free);\n>  \t/* Return Congestion Notification only if we dropped a packet\n>  \t * from this flow.\n>  \t */\n> @@ -675,7 +679,7 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt)\n>  \n>  \tqlen = sch->q.qlen;\n>  \twhile (sch->q.qlen > q->limit)\n> -\t\tdropped += sfq_drop(sch);\n> +\t\tdropped += sfq_drop(sch, NULL);\n>  \tqdisc_tree_reduce_backlog(sch, qlen - sch->q.qlen, dropped);\n>  \n>  \tdel_timer(&q->perturb_timer);","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Ugmrn50G\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xf1Fz3wXLz9sPm\n\tfor <patchwork-incoming@ozlabs.org>;\n\tFri, 25 Aug 2017 22:49:43 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1754800AbdHYMtl (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tFri, 25 Aug 2017 08:49:41 -0400","from mail-pg0-f67.google.com ([74.125.83.67]:36571 \"EHLO\n\tmail-pg0-f67.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1754704AbdHYMtk (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Fri, 25 Aug 2017 08:49:40 -0400","by mail-pg0-f67.google.com with SMTP id 83so3772496pgb.3\n\tfor <netdev@vger.kernel.org>; Fri, 25 Aug 2017 05:49:40 -0700 (PDT)","from [192.168.86.171] (c-67-180-167-114.hsd1.ca.comcast.net.\n\t[67.180.167.114]) by smtp.googlemail.com with ESMTPSA id\n\tz189sm9596246pgb.12.2017.08.25.05.49.38\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tFri, 25 Aug 2017 05:49:38 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=from:message-id:subject:to:cc:date:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=XndAnU8Zk6W4tXlG0cYIzQje7X8RQzxtjuZJEjWNBCk=;\n\tb=Ugmrn50GE+s3FtJiMy0ncoltFpP63a8tpUNSf8rubuev71yYuVvNwplNGOuTDt84Uo\n\thB0SHatq6pGgVRTVAhOVL69Vkbob8fgKdZ3Hc7LXrlIJ6oAJzErAifOTX432LhkKcAfG\n\tKebi9yskH39rbb3KOEa0MyJa7MvDVI1brjaYsQgmyNcWxVM5bb1ikyTbP/DZGNr5Xb38\n\tHiZ0F4ZWS/aDuuetfLGqQKmhUwj8l0qX9Oijwcag8vG4iwWZOqJs5kSK/CUT6po2blfd\n\t8qJzLfOaLKf6C6WyDbFlRTa3GQ+G8xoFexE5MCZrlPWHS6qRJB7GYq5ojSgtq+7FxS4L\n\tGILg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:message-id:subject:to:cc:date:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=XndAnU8Zk6W4tXlG0cYIzQje7X8RQzxtjuZJEjWNBCk=;\n\tb=EETeyiLU4u4ut+lAcLlxmOXZhR6LIak5DwnmfcFz+YNzNXgamMNAjeoW2LuDOk8RQC\n\tvJdliTcOf5yE3kXcrfTshvKoN6r7N2lPt4z7D7UUlzaOnrWYhuLkEqjoIMxw0R7nSJX+\n\tf+raf6FXn/3Z6um5drhMfSewxnDw/wZYitGeTXncDgUgRDI2ru5YOCd752PL5iGPvU8D\n\t0IEc8+xhDHUWvhsLYYAcURfj2xhdNQzRde2Hhf59yU0rM/25+aaTQ9glR6YkjUc/hIY0\n\tn0UMaBt+etVpNw6qbAuCAvinIC9Hc0kBGDzutAxuCE5BHd0XZiGnPMIaBrL/88sBRnxV\n\tdFog==","X-Gm-Message-State":"AHYfb5gsTp8DJwYj6V5HOMt7c2mLLCshzOJA+mNlpuTaEvXTpjorbfTg\n\twwCe3pWD1TQcVKdU","X-Received":"by 10.99.186.67 with SMTP id l3mr9781329pgu.272.1503665379859;\n\tFri, 25 Aug 2017 05:49:39 -0700 (PDT)","From":"Eric Dumazet <erdnetdev@gmail.com>","X-Google-Original-From":"Eric Dumazet <erdnetdev@googlemail.com>","Message-ID":"<1503665377.18816.2.camel@edumazet-glaptop3.roam.corp.google.com>","Subject":"Re: [PATCH net-next RESEND] sched: sfq: drop packets after root\n\tqdisc lock is released","To":"gfree.wind@vip.163.com","Cc":"jhs@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us,\n\tdavem@davemloft.net, edumazet@google.com, netdev@vger.kernel.org","Date":"Fri, 25 Aug 2017 05:49:37 -0700","In-Reply-To":"<1503646997-94678-1-git-send-email-gfree.wind@vip.163.com>","References":"<1503646997-94678-1-git-send-email-gfree.wind@vip.163.com>","Content-Type":"text/plain; charset=\"UTF-8\"","X-Mailer":"Evolution 3.10.4-0ubuntu2 ","Mime-Version":"1.0","Content-Transfer-Encoding":"7bit","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}}]