[{"id":1773568,"web_url":"http://patchwork.ozlabs.org/comment/1773568/","msgid":"<20170922125541.GA2005@nanopsycho.orion>","list_archive_url":null,"date":"2017-09-22T12:55:41","subject":"Re: [PATCH net-next 10/10] net: hns3: Add mqprio support when\n\tinteracting with network stack","submitter":{"id":15321,"url":"http://patchwork.ozlabs.org/api/people/15321/","name":"Jiri Pirko","email":"jiri@resnulli.us"},"content":"Thu, Sep 21, 2017 at 01:21:53PM CEST, linyunsheng@huawei.com wrote:\n>When using tc qdisc to configure DCB parameter, dcb_ops->setup_tc\n>is used to tell hclge_dcb module to do the setup.\n>When using lldptool to configure DCB parameter, hclge_dcb module\n>call the client_ops->setup_tc to tell network stack which queue\n>and priority is using for specific tc.\n>\n>Signed-off-by: Yunsheng Lin \n\n[...]\n\t\n\t\n\t\n>-static int hns3_setup_tc(struct net_device *netdev, u8 tc)\n>+static int hns3_setup_tc(struct net_device *netdev, u8 tc, u8 *prio_tc)\n> {\n> \tstruct hns3_nic_priv *priv = netdev_priv(netdev);\n> \tstruct hnae3_handle *h = priv->ae_handle;\n> \tstruct hnae3_knic_private_info *kinfo = &h->kinfo;\n>+\tbool if_running = netif_running(netdev);\n> \tunsigned int i;\n> \tint ret;\n> \n> \tif (tc > HNAE3_MAX_TC)\n> \t\treturn -EINVAL;\n> \n>-\tif (kinfo->num_tc == tc)\n>-\t\treturn 0;\n>-\n> \tif (!netdev)\n> \t\treturn -EINVAL;\n> \n>-\tif (!tc) {\n>+\tif (if_running) {\n>+\t\t(void)hns3_nic_net_stop(netdev);\n>+\t\tmsleep(100);\n>+\t}\n>+\n>+\tret = (kinfo->dcb_ops && kinfo->dcb_ops->setup_tc) ?\n>+\t\tkinfo->dcb_ops->setup_tc(h, tc, prio_tc) : -EOPNOTSUPP;\n\nThis is most odd. Why do you call dcb_ops from ndo_setup_tc callback?\nWhy are you mixing this together? prio->tc mapping can be done\ndirectly in dcbnl","headers":{"Return-Path":"","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=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=resnulli-us.20150623.gappssmtp.com\n\theader.i=@resnulli-us.20150623.gappssmtp.com\n\theader.b=\"MGYAh9UP\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xzD446Nfrz9s9Y\n\tfor ;\n\tFri, 22 Sep 2017 22:55:48 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752321AbdIVMzq (ORCPT );\n\tFri, 22 Sep 2017 08:55:46 -0400","from mail-wr0-f195.google.com ([209.85.128.195]:38444 \"EHLO\n\tmail-wr0-f195.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752196AbdIVMzo (ORCPT\n\t); Fri, 22 Sep 2017 08:55:44 -0400","by mail-wr0-f195.google.com with SMTP id p37so605034wrb.5\n\tfor ; Fri, 22 Sep 2017 05:55:43 -0700 (PDT)","from localhost (jirka.pirko.cz. [84.16.102.26])\n\tby smtp.gmail.com with ESMTPSA id\n\tn9sm4601558wmd.12.2017.09.22.05.55.42\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 22 Sep 2017 05:55:42 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=resnulli-us.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=n+2hhAjiWrg6ROzUAWocWfeB8VNBI06Hq7RdJCZu9/k=;\n\tb=MGYAh9UPUrd3xiq7npmJ0G6B/6VZQZcVcFZd57JRXi1tT120kP/xyZfq9amuUO51UF\n\t3kjDseLn84yx+G5v39hAcUDNfIAsXhbks9Yrcq0E/3Lg9PjO8XMBzK44jJVI6pzkbIPH\n\tFLAqWE4eMhd6bwVdzyd4mF5dQt6D51wJq2dTzcp1Q8sYgAEBoSWBxKw976cFLCTKzIpn\n\tAUVrEfxnVR/q4D26XEhDc/i+3OqcHWNCTyaTWrQwxY3Ya/DOrNVUWjWNEVwCs7qfbve8\n\tkqj3R3nEc/66VZUWEQQNszjFnXRO9Z7RUqKjvb51y6dBSm8b9TppaK8n9X68i4xAjZoD\n\tQftg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=n+2hhAjiWrg6ROzUAWocWfeB8VNBI06Hq7RdJCZu9/k=;\n\tb=chOnI13Qm2o0fqtTLrjl6pZ6eUhyNPW8nXEQ33USYfg1ib0WYlYm2EG/MrsiIDhGd0\n\tKwIxr9poxSbK/EwYQaMi6Y418svK3bY2/i5Zf9fZOJwGRSxPtfhfDZCfgUelww+rQkDi\n\tMYfvTFMhhaEZio2KWA0yaZwtIJrZVfKfwNTjnuOykXUp15UGlB1+6RiRbBZDhP+hEi+k\n\t9Iyp7AnLY/fuBqt+qzW063vDT73JZmF9HGmfpk4GVVjB//EkocRmocWiacYc8FNVj72p\n\tbq43vl23lQBl/Sqel32FxlH9xh5eZsODJkY7dC2nNiLatmSNsRpBVhZfukZzsdgtd8gI\n\tmMdQ==","X-Gm-Message-State":"AHPjjUiGACFzI1Iom8F4Skr2Rz/DpksXqngtJ+HUPY/HhVsv3Ti4K7Bf\n\tHsuzz/QJ+Y4DN6pCiJ5fef+qdg==","X-Google-Smtp-Source":"AOwi7QBuOcwnuLoFtFmstZN+NeSZGE/CHrVRIQP2viEhJcXgkHNbftdJx3BAn2JuDh+ggJl97yur/Q==","X-Received":"by 10.223.157.198 with SMTP id q6mr4764584wre.102.1506084942907; \n\tFri, 22 Sep 2017 05:55:42 -0700 (PDT)","Date":"Fri, 22 Sep 2017 14:55:41 +0200","From":"Jiri Pirko ","To":"Yunsheng Lin ","Cc":"davem@davemloft.net, huangdaode@hisilicon.com,\n\txuwei5@hisilicon.com, liguozhu@hisilicon.com,\n\tYisen.Zhuang@huawei.com, gabriele.paoloni@huawei.com,\n\tjohn.garry@huawei.com, linuxarm@huawei.com, salil.mehta@huawei.com,\n\tlipeng321@huawei.com, netdev@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org","Subject":"Re: [PATCH net-next 10/10] net: hns3: Add mqprio support when\n\tinteracting with network stack","Message-ID":"<20170922125541.GA2005@nanopsycho.orion>","References":"<1505992913-107256-1-git-send-email-linyunsheng@huawei.com>\n\t<1505992913-107256-11-git-send-email-linyunsheng@huawei.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<1505992913-107256-11-git-send-email-linyunsheng@huawei.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1773713,"web_url":"http://patchwork.ozlabs.org/comment/1773713/","msgid":"<20170922160322.GB2005@nanopsycho.orion>","list_archive_url":null,"date":"2017-09-22T16:03:22","subject":"Re: [PATCH net-next 10/10] net: hns3: Add mqprio support when\n\tinteracting with network stack","submitter":{"id":15321,"url":"http://patchwork.ozlabs.org/api/people/15321/","name":"Jiri Pirko","email":"jiri@resnulli.us"},"content":"Fri, Sep 22, 2017 at 04:11:51PM CEST, linyunsheng@huawei.com wrote:\n>Hi, Jiri\n>\n>>>- if (!tc) {\n>>>+ if (if_running) {\n>>>+ (void)hns3_nic_net_stop(netdev);\n>>>+ msleep(100);\n>>>+ }\n>>>+\n>>>+ ret = (kinfo->dcb_ops && kinfo->dcb_ops->>setup_tc) ?\n>>>+ kinfo->dcb_ops->setup_tc(h, tc, prio_tc) : ->EOPNOTSUPP;\n>\n>>This is most odd. Why do you call dcb_ops from >ndo_setup_tc callback?\n>>Why are you mixing this together? prio->tc mapping >can be done\n>>directly in dcbnl\n>\n>Here is what we do in dcb_ops->setup_tc:\n>Firstly, if current tc num is different from the tc num\n>that user provide, then we setup the queues for each\n>tc.\n>\n>Secondly, we tell hardware the pri to tc mapping that\n>the stack is using. In rx direction, our hardware need\n>that mapping to put different packet into different tc'\n>queues according to the priority of the packet, then\n>rss decides which specific queue in the tc should the\n>packet goto.\n>\n>By mixing, I suppose you meant why we need the\n>pri to tc infomation?\n\nby mixing, I mean what I wrote. You are calling dcb_ops callback from\nndo_setup_tc callback. So you are mixing DCBNL subsystem and TC\nsubsystem. Why? Why do you need sch_mqprio? Why DCBNL is not enough for\nall?\n\n\n\n>I hope I did not misunderstand your question, thanks\n>for your time reviewing.","headers":{"Return-Path":"","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=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=resnulli-us.20150623.gappssmtp.com\n\theader.i=@resnulli-us.20150623.gappssmtp.com\n\theader.b=\"oyk+S5zm\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xzJDq5vWfz9s83\n\tfor ;\n\tSat, 23 Sep 2017 02:03:39 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752259AbdIVQD0 (ORCPT );\n\tFri, 22 Sep 2017 12:03:26 -0400","from mail-wm0-f42.google.com ([74.125.82.42]:43922 \"EHLO\n\tmail-wm0-f42.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752078AbdIVQDY (ORCPT\n\t); Fri, 22 Sep 2017 12:03:24 -0400","by mail-wm0-f42.google.com with SMTP id m72so2653266wmc.0\n\tfor ; Fri, 22 Sep 2017 09:03:24 -0700 (PDT)","from localhost (jirka.pirko.cz. [84.16.102.26])\n\tby smtp.gmail.com with ESMTPSA id\n\ts3sm116431wrc.24.2017.09.22.09.03.22\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 22 Sep 2017 09:03:22 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=resnulli-us.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=TyE8nF3z6pX1qVMjjjJ7S2YGtEmwQBlDAuNlm+GHtNI=;\n\tb=oyk+S5zm/ZIEmFPkfkLlBTvaFGiRNk62hO+3o037suNoBGlk88BkDGk7Drzs/zUp4M\n\ttDEf9Fbm5dqkXd3++AV4jKJOa/GIlQzi1k3rDIbNfoZ4qx2hA013fcTSF/D1n9DD+xY4\n\tbEoKVYVgkzV4erDPWvH7rg17ZS0vVeD8JwsAFLsWV9jkXXUM2PCSetqT2uo5fhzYztB0\n\txfHckI3oTgeC/oKM8faQs3suxERlZyfP6pINT9JCkBx5JWhbG6bAcFMTlVJ99rC2EbHU\n\tus8GSUjo43NegXtrXeyreTHp2awM+kr62QLeCRuJn1QdvEk7osvv93IeHkHTwVV4gLB3\n\tmshg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=TyE8nF3z6pX1qVMjjjJ7S2YGtEmwQBlDAuNlm+GHtNI=;\n\tb=m3JWQtibStfshc1N40tIHxEKfu5CDNhBkxv3qYa9KjxRWBWUi5HNLe8bNLe9++pk19\n\tPryCInOOJGptkHffSZX7fismrXkBV224hLoIZQFKyjCpM1PSPWUNPRat6h4lTy97USvT\n\tF5TjTLKRhx8tIOXPT+Jl9unqfv8zltNPyInc56MB6VEMaoErzDCnrdGO8cE0xHE2ST3Z\n\tNdMn0L5zuI+GGOIEwKleN2Uy5DRJ4v4eIafkyxURReVlqkvCYfG6ufQ7lV35L8GXxeue\n\t/IEgUj+Gy5suYCf1vCbebWJxGyIS96os0FTxk3ZMxYYg5UeMWK3yBuFu4TbcBd2PBtTF\n\tm6jg==","X-Gm-Message-State":"AHPjjUjMK1f3REz3ufw4Ko9pgq34hTbB+JHu1rg31K0PZAwDhLoXi2Hq\n\t2k9TzpGhkF50+djiKrLN91fojQ==","X-Google-Smtp-Source":"AOwi7QD70riDlaukhtC7dknwdPzgnt2xnndSFrAPElFBaleORYWSSx5o2XCBOumrq+hYqm37qdp8/w==","X-Received":"by 10.28.70.133 with SMTP id t127mr4723326wma.42.1506096203540; \n\tFri, 22 Sep 2017 09:03:23 -0700 (PDT)","Date":"Fri, 22 Sep 2017 18:03:22 +0200","From":"Jiri Pirko ","To":"linyunsheng ","Cc":"\"davem@davemloft.net\" ,\n\thuangdaode ,\n\t\"xuwei (O)\" ,\n\t\"Liguozhu (Kenneth)\" ,\n\t\"Zhuangyuzeng (Yisen)\" ,\n\tGabriele Paoloni ,\n\tJohn Garry , Linuxarm ,\n\tSalil Mehta ,\n\t\"lipeng (Y)\" ,\n\t\"netdev@vger.kernel.org\" ,\n\t\"linux-kernel@vger.kernel.org\" ","Subject":"Re: [PATCH net-next 10/10] net: hns3: Add mqprio support when\n\tinteracting with network stack","Message-ID":"<20170922160322.GB2005@nanopsycho.orion>","References":"<1505992913-107256-1-git-send-email-linyunsheng@huawei.com>\n\t<1505992913-107256-11-git-send-email-linyunsheng@huawei.com>\n\t<20170922125541.GA2005@nanopsycho.orion>\n\t<59c51a37.a1c4df0a.ac4e2.8df0SMTPIN_ADDED_BROKEN@mx.google.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<59c51a37.a1c4df0a.ac4e2.8df0SMTPIN_ADDED_BROKEN@mx.google.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1773947,"web_url":"http://patchwork.ozlabs.org/comment/1773947/","msgid":"","list_archive_url":null,"date":"2017-09-23T00:47:20","subject":"Re: [PATCH net-next 10/10] net: hns3: Add mqprio support when\n\tinteracting with network stack","submitter":{"id":71804,"url":"http://patchwork.ozlabs.org/api/people/71804/","name":"Yunsheng Lin","email":"linyunsheng@huawei.com"},"content":"Hi, Jiri\n\nOn 2017/9/23 0:03, Jiri Pirko wrote:\n> Fri, Sep 22, 2017 at 04:11:51PM CEST, linyunsheng@huawei.com wrote:\n>> Hi, Jiri\n>>\n>>>> - if (!tc) {\n>>>> + if (if_running) {\n>>>> + (void)hns3_nic_net_stop(netdev);\n>>>> + msleep(100);\n>>>> + }\n>>>> +\n>>>> + ret = (kinfo->dcb_ops && kinfo->dcb_ops->>setup_tc) ?\n>>>> + kinfo->dcb_ops->setup_tc(h, tc, prio_tc) : ->EOPNOTSUPP;\n>>\n>>> This is most odd. Why do you call dcb_ops from >ndo_setup_tc callback?\n>>> Why are you mixing this together? prio->tc mapping >can be done\n>>> directly in dcbnl\n>>\n>> Here is what we do in dcb_ops->setup_tc:\n>> Firstly, if current tc num is different from the tc num\n>> that user provide, then we setup the queues for each\n>> tc.\n>>\n>> Secondly, we tell hardware the pri to tc mapping that\n>> the stack is using. In rx direction, our hardware need\n>> that mapping to put different packet into different tc'\n>> queues according to the priority of the packet, then\n>> rss decides which specific queue in the tc should the\n>> packet goto.\n>>\n>> By mixing, I suppose you meant why we need the\n>> pri to tc infomation?\n> \n> by mixing, I mean what I wrote. You are calling dcb_ops callback from\n> ndo_setup_tc callback. So you are mixing DCBNL subsystem and TC\n> subsystem. Why? Why do you need sch_mqprio? Why DCBNL is not enough for\n> all?\n\nWhen using lldptool, dcbnl is involved.\n\nBut when using tc qdisc, dcbbl is not involved, below is the a few key\ncall graph in the kernel when tc qdisc cmd is executed.\n\ncmd:\ntc qdisc add dev eth0 root handle 1:0 mqprio num_tc 4 map 1 2 3 3 1 3 1 1 hw 1\n\ncall graph:\nrtnetlink_rcv_msg -> tc_modify_qdisc -> qdisc_create -> mqprio_init ->\nhns3_nic_setup_tc\n\nWhen hns3_nic_setup_tc is called, we need to know how many tc num and\nprio_tc mapping from the tc_mqprio_qopt which is provided in the paramter\nin the ndo_setup_tc function, and dcb_ops is the our hardware specific\nmethod to setup the tc related parameter to the hardware, so this is why\nwe call dcb_ops callback in ndo_setup_tc callback.\n\nI hope this will answer your question, thanks for your time.\n\n> \n> \n> \n>> I hope I did not misunderstand your question, thanks\n>> for your time reviewing.\n> \n> .\n>","headers":{"Return-Path":"","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=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xzWsl5Ktgz9sRm\n\tfor ;\n\tSat, 23 Sep 2017 10:47:55 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752399AbdIWArn (ORCPT );\n\tFri, 22 Sep 2017 20:47:43 -0400","from szxga04-in.huawei.com ([45.249.212.190]:6981 \"EHLO\n\tszxga04-in.huawei.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752004AbdIWArm (ORCPT\n\t); Fri, 22 Sep 2017 20:47:42 -0400","from 172.30.72.59 (EHLO DGGEMS414-HUB.china.huawei.com)\n\t([172.30.72.59])\n\tby dggrg04-dlp.huawei.com (MOS 4.4.6-GA FastPath queued)\n\twith ESMTP id DHT89890; Sat, 23 Sep 2017 08:47:35 +0800 (CST)","from [127.0.0.1] (10.74.191.121) by DGGEMS414-HUB.china.huawei.com\n\t(10.3.19.214) with Microsoft SMTP Server id 14.3.301.0;\n\tSat, 23 Sep 2017 08:47:26 +0800"],"Subject":"Re: [PATCH net-next 10/10] net: hns3: Add mqprio support when\n\tinteracting with network stack","To":"Jiri Pirko ","CC":"\"davem@davemloft.net\" ,\n\thuangdaode ,\n\t\"xuwei (O)\" ,\n\t\"Liguozhu (Kenneth)\" ,\n\t\"Zhuangyuzeng (Yisen)\" ,\n\tGabriele Paoloni ,\n\tJohn Garry , Linuxarm ,\n\t\"Salil Mehta\" ,\n\t\"lipeng (Y)\" ,\n\t\"netdev@vger.kernel.org\" ,\n\t\"linux-kernel@vger.kernel.org\" ","References":"<1505992913-107256-1-git-send-email-linyunsheng@huawei.com>\n\t<1505992913-107256-11-git-send-email-linyunsheng@huawei.com>\n\t<20170922125541.GA2005@nanopsycho.orion>\n\t<59c51a37.a1c4df0a.ac4e2.8df0SMTPIN_ADDED_BROKEN@mx.google.com>\n\t<20170922160322.GB2005@nanopsycho.orion>","From":"Yunsheng Lin ","Message-ID":"","Date":"Sat, 23 Sep 2017 08:47:20 +0800","User-Agent":"Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.0","MIME-Version":"1.0","In-Reply-To":"<20170922160322.GB2005@nanopsycho.orion>","Content-Type":"text/plain; charset=\"utf-8\"","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[10.74.191.121]","X-CFilter-Loop":"Reflected","X-Mirapoint-Virus-RAPID-Raw":"score=unknown(0),\n\trefid=str=0001.0A020205.59C5AF28.002B, ss=1, re=0.000, recu=0.000,\n\treip=0.000, cl=1, cld=1, fgs=0, ip=0.0.0.0,\n\tso=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32","X-Mirapoint-Loop-Id":"242dd474bed440e20ef05668cc3d6624","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1774136,"web_url":"http://patchwork.ozlabs.org/comment/1774136/","msgid":"<20170924113724.GA2029@nanopsycho>","list_archive_url":null,"date":"2017-09-24T11:37:24","subject":"Re: [PATCH net-next 10/10] net: hns3: Add mqprio support when\n\tinteracting with network stack","submitter":{"id":15321,"url":"http://patchwork.ozlabs.org/api/people/15321/","name":"Jiri Pirko","email":"jiri@resnulli.us"},"content":"Sat, Sep 23, 2017 at 02:47:20AM CEST, linyunsheng@huawei.com wrote:\n>Hi, Jiri\n>\n>On 2017/9/23 0:03, Jiri Pirko wrote:\n>> Fri, Sep 22, 2017 at 04:11:51PM CEST, linyunsheng@huawei.com wrote:\n>>> Hi, Jiri\n>>>\n>>>>> - if (!tc) {\n>>>>> + if (if_running) {\n>>>>> + (void)hns3_nic_net_stop(netdev);\n>>>>> + msleep(100);\n>>>>> + }\n>>>>> +\n>>>>> + ret = (kinfo->dcb_ops && kinfo->dcb_ops->>setup_tc) ?\n>>>>> + kinfo->dcb_ops->setup_tc(h, tc, prio_tc) : ->EOPNOTSUPP;\n>>>\n>>>> This is most odd. Why do you call dcb_ops from >ndo_setup_tc callback?\n>>>> Why are you mixing this together? prio->tc mapping >can be done\n>>>> directly in dcbnl\n>>>\n>>> Here is what we do in dcb_ops->setup_tc:\n>>> Firstly, if current tc num is different from the tc num\n>>> that user provide, then we setup the queues for each\n>>> tc.\n>>>\n>>> Secondly, we tell hardware the pri to tc mapping that\n>>> the stack is using. In rx direction, our hardware need\n>>> that mapping to put different packet into different tc'\n>>> queues according to the priority of the packet, then\n>>> rss decides which specific queue in the tc should the\n>>> packet goto.\n>>>\n>>> By mixing, I suppose you meant why we need the\n>>> pri to tc infomation?\n>> \n>> by mixing, I mean what I wrote. You are calling dcb_ops callback from\n>> ndo_setup_tc callback. So you are mixing DCBNL subsystem and TC\n>> subsystem. Why? Why do you need sch_mqprio? Why DCBNL is not enough for\n>> all?\n>\n>When using lldptool, dcbnl is involved.\n>\n>But when using tc qdisc, dcbbl is not involved, below is the a few key\n>call graph in the kernel when tc qdisc cmd is executed.\n>\n>cmd:\n>tc qdisc add dev eth0 root handle 1:0 mqprio num_tc 4 map 1 2 3 3 1 3 1 1 hw 1\n>\n>call graph:\n>rtnetlink_rcv_msg -> tc_modify_qdisc -> qdisc_create -> mqprio_init ->\n>hns3_nic_setup_tc\n>\n>When hns3_nic_setup_tc is called, we need to know how many tc num and\n>prio_tc mapping from the tc_mqprio_qopt which is provided in the paramter\n>in the ndo_setup_tc function, and dcb_ops is the our hardware specific\n>method to setup the tc related parameter to the hardware, so this is why\n>we call dcb_ops callback in ndo_setup_tc callback.\n>\n>I hope this will answer your question, thanks for your time.\n\nOkay. I understand that you have a usecase for mqprio mapping offload\nwithout lldptool being involved. Ok. I believe it is wrong to call dcb_ops\nfrom tc callback. You should have a generic layer inside the driver and\ncall it from both dcb_ops and tc callbacks.\n\nAlso, what happens If I run lldptool concurrently with mqprio? Who wins\nand is going to configure the mapping?\n\n\n>\n>> \n>> \n>> \n>>> I hope I did not misunderstand your question, thanks\n>>> for your time reviewing.\n>> \n>> .\n>> \n>","headers":{"Return-Path":"","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=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=resnulli-us.20150623.gappssmtp.com\n\theader.i=@resnulli-us.20150623.gappssmtp.com\n\theader.b=\"t+8ykQ4P\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y0QF361ncz9sxR\n\tfor ;\n\tSun, 24 Sep 2017 21:37:43 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752143AbdIXLh3 (ORCPT );\n\tSun, 24 Sep 2017 07:37:29 -0400","from mail-wr0-f194.google.com ([209.85.128.194]:33101 \"EHLO\n\tmail-wr0-f194.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752004AbdIXLh1 (ORCPT\n\t); Sun, 24 Sep 2017 07:37:27 -0400","by mail-wr0-f194.google.com with SMTP id b9so2871174wra.0\n\tfor ; Sun, 24 Sep 2017 04:37:26 -0700 (PDT)","from localhost (ip-89-177-136-69.net.upcbroadband.cz.\n\t[89.177.136.69]) by smtp.gmail.com with ESMTPSA id\n\tb55sm874616wrb.11.2017.09.24.04.37.25\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSun, 24 Sep 2017 04:37:25 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=resnulli-us.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=7kJ5twsdKjUC2CjaLe+RHcSNzH1fPNIxSLor9Qa5VTY=;\n\tb=t+8ykQ4PPs5Psxq3cKkt4JA0gAa/5j5zKls4REDBIa5t+aJ4HNZUMxlSQbr4HDYeQ/\n\tVBa5cxclOY+0ji916p/9SZ9lN69cdI2zCyD37LAGZGz8eQ64iUwQB1wZ3TD1+N8GvLDy\n\tMjzP76xyqYoO9B+lmc/UKj2o4N56+J+K6eKmDv2Heii1HPsq/PuvjJEc5HBcinMPAfcQ\n\tHUV/8AgJlU/fNdsdAPlKWpvf684RQRt+1/UtZ3GagnxobGue8+C+hba+x/EaOfJNTouU\n\tttHnz/Xg6l1PCRj1Ta5bMi8Efifkh1/Q0eF17KWu8C+YRe1K1Uctd6ZiSwYjhILSX28s\n\tvLhA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=7kJ5twsdKjUC2CjaLe+RHcSNzH1fPNIxSLor9Qa5VTY=;\n\tb=MIQKaIK5HS+NGj8mAuo2oeKVlaozBWGgRJGLjnvt5zdWT6ivDu4ZYFXRYCMpj8CY2r\n\tIyqqjIejgUKXGpMvROfnfqlBuWyDY5PLtkrrBSrjFfpXjNDwFx2fHr7TN64kn5NNJ64V\n\twh2CJEyaeQ4Y/sJcEc5eE1jcfCF3s3QjMQ+dGxbX/BYjQ1VUETjT/H69DcKFW3pVbap2\n\ttWNEt098qz4hUPQm8dHd8oUzjWUBjuNlQi/BUsi7P4f8/rcfnCDKDOQlOogbbvy/d5gR\n\t+ok+mvFyyX6UU+4vUr9PnUeFIhtHja/bDFd/c7uuS5WDp7w5e9F9y8y3p/hVyhHCspOl\n\tG1GA==","X-Gm-Message-State":"AHPjjUgnQyWAh49nNFnL81VjmP7sE/3eu4o1OUzJIE9feFPxQeXI0gaI\n\tWv1J4x5PVqGcIvxRDH3r96soKA==","X-Google-Smtp-Source":"AOwi7QDVn9Ls/UXbIs02W4jaxrpbO1yZizhmr9zFAe/877PlFQjK0+uYDWFWS939Yr8hJ19o/Ai4qA==","X-Received":"by 10.223.173.204 with SMTP id w70mr3823312wrc.281.1506253045954;\n\tSun, 24 Sep 2017 04:37:25 -0700 (PDT)","Date":"Sun, 24 Sep 2017 13:37:24 +0200","From":"Jiri Pirko ","To":"Yunsheng Lin ","Cc":"\"davem@davemloft.net\" ,\n\thuangdaode ,\n\t\"xuwei (O)\" ,\n\t\"Liguozhu (Kenneth)\" ,\n\t\"Zhuangyuzeng (Yisen)\" ,\n\tGabriele Paoloni ,\n\tJohn Garry , Linuxarm ,\n\tSalil Mehta ,\n\t\"lipeng (Y)\" ,\n\t\"netdev@vger.kernel.org\" ,\n\t\"linux-kernel@vger.kernel.org\" ","Subject":"Re: [PATCH net-next 10/10] net: hns3: Add mqprio support when\n\tinteracting with network stack","Message-ID":"<20170924113724.GA2029@nanopsycho>","References":"<1505992913-107256-1-git-send-email-linyunsheng@huawei.com>\n\t<1505992913-107256-11-git-send-email-linyunsheng@huawei.com>\n\t<20170922125541.GA2005@nanopsycho.orion>\n\t<59c51a37.a1c4df0a.ac4e2.8df0SMTPIN_ADDED_BROKEN@mx.google.com>\n\t<20170922160322.GB2005@nanopsycho.orion>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1774325,"web_url":"http://patchwork.ozlabs.org/comment/1774325/","msgid":"<290b0679-bfc2-c23c-00ee-43768c1c2327@huawei.com>","list_archive_url":null,"date":"2017-09-25T00:45:08","subject":"Re: [PATCH net-next 10/10] net: hns3: Add mqprio support when\n\tinteracting with network stack","submitter":{"id":71804,"url":"http://patchwork.ozlabs.org/api/people/71804/","name":"Yunsheng Lin","email":"linyunsheng@huawei.com"},"content":"Hi, Jiri\n\nOn 2017/9/24 19:37, Jiri Pirko wrote:\n> Sat, Sep 23, 2017 at 02:47:20AM CEST, linyunsheng@huawei.com wrote:\n>> Hi, Jiri\n>>\n>> On 2017/9/23 0:03, Jiri Pirko wrote:\n>>> Fri, Sep 22, 2017 at 04:11:51PM CEST, linyunsheng@huawei.com wrote:\n>>>> Hi, Jiri\n>>>>\n>>>>>> - if (!tc) {\n>>>>>> + if (if_running) {\n>>>>>> + (void)hns3_nic_net_stop(netdev);\n>>>>>> + msleep(100);\n>>>>>> + }\n>>>>>> +\n>>>>>> + ret = (kinfo->dcb_ops && kinfo->dcb_ops->>setup_tc) ?\n>>>>>> + kinfo->dcb_ops->setup_tc(h, tc, prio_tc) : ->EOPNOTSUPP;\n>>>>\n>>>>> This is most odd. Why do you call dcb_ops from >ndo_setup_tc callback?\n>>>>> Why are you mixing this together? prio->tc mapping >can be done\n>>>>> directly in dcbnl\n>>>>\n>>>> Here is what we do in dcb_ops->setup_tc:\n>>>> Firstly, if current tc num is different from the tc num\n>>>> that user provide, then we setup the queues for each\n>>>> tc.\n>>>>\n>>>> Secondly, we tell hardware the pri to tc mapping that\n>>>> the stack is using. In rx direction, our hardware need\n>>>> that mapping to put different packet into different tc'\n>>>> queues according to the priority of the packet, then\n>>>> rss decides which specific queue in the tc should the\n>>>> packet goto.\n>>>>\n>>>> By mixing, I suppose you meant why we need the\n>>>> pri to tc infomation?\n>>>\n>>> by mixing, I mean what I wrote. You are calling dcb_ops callback from\n>>> ndo_setup_tc callback. So you are mixing DCBNL subsystem and TC\n>>> subsystem. Why? Why do you need sch_mqprio? Why DCBNL is not enough for\n>>> all?\n>>\n>> When using lldptool, dcbnl is involved.\n>>\n>> But when using tc qdisc, dcbbl is not involved, below is the a few key\n>> call graph in the kernel when tc qdisc cmd is executed.\n>>\n>> cmd:\n>> tc qdisc add dev eth0 root handle 1:0 mqprio num_tc 4 map 1 2 3 3 1 3 1 1 hw 1\n>>\n>> call graph:\n>> rtnetlink_rcv_msg -> tc_modify_qdisc -> qdisc_create -> mqprio_init ->\n>> hns3_nic_setup_tc\n>>\n>> When hns3_nic_setup_tc is called, we need to know how many tc num and\n>> prio_tc mapping from the tc_mqprio_qopt which is provided in the paramter\n>> in the ndo_setup_tc function, and dcb_ops is the our hardware specific\n>> method to setup the tc related parameter to the hardware, so this is why\n>> we call dcb_ops callback in ndo_setup_tc callback.\n>>\n>> I hope this will answer your question, thanks for your time.\n> \n> Okay. I understand that you have a usecase for mqprio mapping offload\n> without lldptool being involved. Ok. I believe it is wrong to call dcb_ops\n> from tc callback. You should have a generic layer inside the driver and\n> call it from both dcb_ops and tc callbacks.\n\nActually, dcb_ops is our generic layer inside the driver.\nBelow is high level architecture:\n\n [ tc qdisc ]\t [ lldpad ]\n | |\n | |\n | |\n [ hns3_enet ] [ hns3_dcbnl ]\n \\ /\n \\ /\n \\ /\n [ hclge_dcb ]\n / \\\n / \\\n / \\\n [ hclgc_main ] [ hclge_tm ]\n\nhns3_enet.c implements the ndo_setup_tc callback.\nhns3_dcbnl.c implements the dcbnl_rtnl_ops for stack's DCBNL system.\nhclge_dcb implements the dcb_ops.\nSo we already have a generic layer that tc and dcbnl all call from.\n\n> \n> Also, what happens If I run lldptool concurrently with mqprio? Who wins\n> and is going to configure the mapping?\n\nBoth lldptool and tc qdisc cmd use rtnl interface provided by stack, so\nthey are both protected by rtnl_lock, so we do not have to do the locking\nin the driver.\n\nThe locking is in rtnetlink_rcv_msg:\n\n\trtnl_lock();\n\thandlers = rtnl_dereference(rtnl_msg_handlers[family]);\n\tif (handlers) {\n\t\tdoit = READ_ONCE(handlers[type].doit);\n\t\tif (doit)\n\t\t\terr = doit(skb, nlh, extack);\n\t}\n\trtnl_unlock();\n\nThanks.\n\n> \n> \n>>\n>>>\n>>>\n>>>\n>>>> I hope I did not misunderstand your question, thanks\n>>>> for your time reviewing.\n>>>\n>>> .\n>>>\n>>\n> \n> .\n>","headers":{"Return-Path":"","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=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y0lkf4pD8z9sRV\n\tfor ;\n\tMon, 25 Sep 2017 10:46:02 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S933379AbdIYApv (ORCPT );\n\tSun, 24 Sep 2017 20:45:51 -0400","from szxga05-in.huawei.com ([45.249.212.191]:6558 \"EHLO\n\tszxga05-in.huawei.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S932553AbdIYApu (ORCPT\n\t); Sun, 24 Sep 2017 20:45:50 -0400","from 172.30.72.60 (EHLO DGGEMS413-HUB.china.huawei.com)\n\t([172.30.72.60])\n\tby dggrg05-dlp.huawei.com (MOS 4.4.6-GA FastPath queued)\n\twith ESMTP id DHY65661; Mon, 25 Sep 2017 08:45:17 +0800 (CST)","from [127.0.0.1] (10.74.191.121) by DGGEMS413-HUB.china.huawei.com\n\t(10.3.19.213) with Microsoft SMTP Server id 14.3.301.0;\n\tMon, 25 Sep 2017 08:45:09 +0800"],"Subject":"Re: [PATCH net-next 10/10] net: hns3: Add mqprio support when\n\tinteracting with network stack","To":"Jiri Pirko ","CC":"\"davem@davemloft.net\" ,\n\thuangdaode ,\n\t\"xuwei (O)\" ,\n\t\"Liguozhu (Kenneth)\" ,\n\t\"Zhuangyuzeng (Yisen)\" ,\n\tGabriele Paoloni ,\n\tJohn Garry , Linuxarm ,\n\t\"Salil Mehta\" ,\n\t\"lipeng (Y)\" ,\n\t\"netdev@vger.kernel.org\" ,\n\t\"linux-kernel@vger.kernel.org\" ","References":"<1505992913-107256-1-git-send-email-linyunsheng@huawei.com>\n\t<1505992913-107256-11-git-send-email-linyunsheng@huawei.com>\n\t<20170922125541.GA2005@nanopsycho.orion>\n\t<59c51a37.a1c4df0a.ac4e2.8df0SMTPIN_ADDED_BROKEN@mx.google.com>\n\t<20170922160322.GB2005@nanopsycho.orion>\n\t\n\t<20170924113724.GA2029@nanopsycho>","From":"Yunsheng Lin ","Message-ID":"<290b0679-bfc2-c23c-00ee-43768c1c2327@huawei.com>","Date":"Mon, 25 Sep 2017 08:45:08 +0800","User-Agent":"Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.0","MIME-Version":"1.0","In-Reply-To":"<20170924113724.GA2029@nanopsycho>","Content-Type":"text/plain; charset=\"utf-8\"","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[10.74.191.121]","X-CFilter-Loop":"Reflected","X-Mirapoint-Virus-RAPID-Raw":"score=unknown(0),\n\trefid=str=0001.0A020203.59C851B7.00FA, ss=1, re=0.000, recu=0.000,\n\treip=0.000, cl=1, cld=1, fgs=0, ip=0.0.0.0,\n\tso=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32","X-Mirapoint-Loop-Id":"7a8eac6dfc757f8856b750674479f950","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1774446,"web_url":"http://patchwork.ozlabs.org/comment/1774446/","msgid":"<20170925065740.GB1899@nanopsycho>","list_archive_url":null,"date":"2017-09-25T06:57:40","subject":"Re: [PATCH net-next 10/10] net: hns3: Add mqprio support when\n\tinteracting with network stack","submitter":{"id":15321,"url":"http://patchwork.ozlabs.org/api/people/15321/","name":"Jiri Pirko","email":"jiri@resnulli.us"},"content":"Mon, Sep 25, 2017 at 02:45:08AM CEST, linyunsheng@huawei.com wrote:\n>Hi, Jiri\n>\n>On 2017/9/24 19:37, Jiri Pirko wrote:\n>> Sat, Sep 23, 2017 at 02:47:20AM CEST, linyunsheng@huawei.com wrote:\n>>> Hi, Jiri\n>>>\n>>> On 2017/9/23 0:03, Jiri Pirko wrote:\n>>>> Fri, Sep 22, 2017 at 04:11:51PM CEST, linyunsheng@huawei.com wrote:\n>>>>> Hi, Jiri\n>>>>>\n>>>>>>> - if (!tc) {\n>>>>>>> + if (if_running) {\n>>>>>>> + (void)hns3_nic_net_stop(netdev);\n>>>>>>> + msleep(100);\n>>>>>>> + }\n>>>>>>> +\n>>>>>>> + ret = (kinfo->dcb_ops && kinfo->dcb_ops->>setup_tc) ?\n>>>>>>> + kinfo->dcb_ops->setup_tc(h, tc, prio_tc) : ->EOPNOTSUPP;\n>>>>>\n>>>>>> This is most odd. Why do you call dcb_ops from >ndo_setup_tc callback?\n>>>>>> Why are you mixing this together? prio->tc mapping >can be done\n>>>>>> directly in dcbnl\n>>>>>\n>>>>> Here is what we do in dcb_ops->setup_tc:\n>>>>> Firstly, if current tc num is different from the tc num\n>>>>> that user provide, then we setup the queues for each\n>>>>> tc.\n>>>>>\n>>>>> Secondly, we tell hardware the pri to tc mapping that\n>>>>> the stack is using. In rx direction, our hardware need\n>>>>> that mapping to put different packet into different tc'\n>>>>> queues according to the priority of the packet, then\n>>>>> rss decides which specific queue in the tc should the\n>>>>> packet goto.\n>>>>>\n>>>>> By mixing, I suppose you meant why we need the\n>>>>> pri to tc infomation?\n>>>>\n>>>> by mixing, I mean what I wrote. You are calling dcb_ops callback from\n>>>> ndo_setup_tc callback. So you are mixing DCBNL subsystem and TC\n>>>> subsystem. Why? Why do you need sch_mqprio? Why DCBNL is not enough for\n>>>> all?\n>>>\n>>> When using lldptool, dcbnl is involved.\n>>>\n>>> But when using tc qdisc, dcbbl is not involved, below is the a few key\n>>> call graph in the kernel when tc qdisc cmd is executed.\n>>>\n>>> cmd:\n>>> tc qdisc add dev eth0 root handle 1:0 mqprio num_tc 4 map 1 2 3 3 1 3 1 1 hw 1\n>>>\n>>> call graph:\n>>> rtnetlink_rcv_msg -> tc_modify_qdisc -> qdisc_create -> mqprio_init ->\n>>> hns3_nic_setup_tc\n>>>\n>>> When hns3_nic_setup_tc is called, we need to know how many tc num and\n>>> prio_tc mapping from the tc_mqprio_qopt which is provided in the paramter\n>>> in the ndo_setup_tc function, and dcb_ops is the our hardware specific\n>>> method to setup the tc related parameter to the hardware, so this is why\n>>> we call dcb_ops callback in ndo_setup_tc callback.\n>>>\n>>> I hope this will answer your question, thanks for your time.\n>> \n>> Okay. I understand that you have a usecase for mqprio mapping offload\n>> without lldptool being involved. Ok. I believe it is wrong to call dcb_ops\n>> from tc callback. You should have a generic layer inside the driver and\n>> call it from both dcb_ops and tc callbacks.\n>\n>Actually, dcb_ops is our generic layer inside the driver.\n>Below is high level architecture:\n>\n> [ tc qdisc ]\t [ lldpad ]\n> | |\n> | |\n> | |\n> [ hns3_enet ] [ hns3_dcbnl ]\n> \\ /\n> \\ /\n> \\ /\n> [ hclge_dcb ]\n> / \\\n> / \\\n> / \\\n> [ hclgc_main ] [ hclge_tm ]\n>\n>hns3_enet.c implements the ndo_setup_tc callback.\n>hns3_dcbnl.c implements the dcbnl_rtnl_ops for stack's DCBNL system.\n>hclge_dcb implements the dcb_ops.\n>So we already have a generic layer that tc and dcbnl all call from.\n>\n>> \n>> Also, what happens If I run lldptool concurrently with mqprio? Who wins\n>> and is going to configure the mapping?\n>\n>Both lldptool and tc qdisc cmd use rtnl interface provided by stack, so\n>they are both protected by rtnl_lock, so we do not have to do the locking\n>in the driver.\n\nI was not asking about locking, which is obvious, I was asking about the\nbehaviour. Like for example:\nIf I use tc to configure some mapping, later on I run lldptool and change\nthe mapping. Does the tc dump show the updated values or the original\nones?\n\n>\n>The locking is in rtnetlink_rcv_msg:\n>\n>\trtnl_lock();\n>\thandlers = rtnl_dereference(rtnl_msg_handlers[family]);\n>\tif (handlers) {\n>\t\tdoit = READ_ONCE(handlers[type].doit);\n>\t\tif (doit)\n>\t\t\terr = doit(skb, nlh, extack);\n>\t}\n>\trtnl_unlock();\n>\n>Thanks.\n>\n>> \n>> \n>>>\n>>>>\n>>>>\n>>>>\n>>>>> I hope I did not misunderstand your question, thanks\n>>>>> for your time reviewing.\n>>>>\n>>>> .\n>>>>\n>>>\n>> \n>> .\n>> \n>","headers":{"Return-Path":"","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=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=resnulli-us.20150623.gappssmtp.com\n\theader.i=@resnulli-us.20150623.gappssmtp.com\n\theader.b=\"o8pQOfR8\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y0vzc2NK1z9tXG\n\tfor ;\n\tMon, 25 Sep 2017 16:57:48 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S932777AbdIYG5o (ORCPT );\n\tMon, 25 Sep 2017 02:57:44 -0400","from mail-wm0-f68.google.com ([74.125.82.68]:33401 \"EHLO\n\tmail-wm0-f68.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S932512AbdIYG5m (ORCPT\n\t); Mon, 25 Sep 2017 02:57:42 -0400","by mail-wm0-f68.google.com with SMTP id m127so6290928wmm.0\n\tfor ; Sun, 24 Sep 2017 23:57:42 -0700 (PDT)","from localhost (ip-89-177-136-69.net.upcbroadband.cz.\n\t[89.177.136.69]) by smtp.gmail.com with ESMTPSA id\n\te77sm16735833wmf.27.2017.09.24.23.57.40\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSun, 24 Sep 2017 23:57:41 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=resnulli-us.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=tUjOYNGfdsIW60VN0wpb8eF5MMY0xTu0pk1jQvEKf2k=;\n\tb=o8pQOfR88s0LDi89nfa4JfuDAdvijMwIHv1wXWSda6eV67Dn9CceYNO1SACICxPGuM\n\tXLoq2KJXpytM8yyhv/ZaLb0ob/RPDHntvXDf1vDN38eY+zu0HBuXQ/Qc90MB3dUIsxm5\n\tnZJxWFZn7fR1Y9yu8HqoPUVucgBrQT2sbdnKW1g7xhJacc0DWZDxhG57eunG8H2lSLk8\n\t76dCyWpkGdM/u8o8mfhsmJMKVCtW7dUMm6T1O6MoS6zy87fKB5rUsyN3+Q0L7hphLOpK\n\t8AJSdpOSuOtP2+80Oe3B6pWRb6VnKLhhZ6xYRkrxtlNdyy7LVV/D2SzKB3qfIyyLmdUD\n\tnetA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=tUjOYNGfdsIW60VN0wpb8eF5MMY0xTu0pk1jQvEKf2k=;\n\tb=luE4VN1CyvCMkWz4QdH03eL5lMWQHoNyudH/3tnICWsnY7QHh2A5S/U0vzcoNkzcq7\n\tat4NXR+NDH5vTm28kPd097dCMSX4Ur7qk7zXe5aFkfx2NG06JHp5uPW+UzUouMutQisL\n\tlVkCd3iioFT9pxpfMldVmuPGlXsShvZR63IDr8gWBBjai3l62GiCXPThPc7Z1SRo7TXl\n\tVPYTZZVChhD+Z2x9o29lTmT7CHWktK9MmHOU4pG7XiDGI4Iqe6i/YugWvJN/hhBU2abr\n\tLbKYiH8z0vUQsd3xXV7QyQvLu+Ts5+0ofYlYjaEnlcfDyBk/vO9Wdu0r+wLAwsVxDQ4L\n\toQkQ==","X-Gm-Message-State":"AHPjjUgC6NBSS44PGAQDIY4tBi84yUZl5yHI0eKgid+IUNe7cr22I1gy\n\tJJDfbfHiAuOQlm0lzga3tn9gMg==","X-Google-Smtp-Source":"AOwi7QAczXffF8rcgQ0ed4uHPAQcD2MJlkJFITEhTkWSOTIb70khabdUA/G0SiwECubG9dUiBN1iEQ==","X-Received":"by 10.28.74.89 with SMTP id x86mr2993357wma.57.1506322661448;\n\tSun, 24 Sep 2017 23:57:41 -0700 (PDT)","Date":"Mon, 25 Sep 2017 08:57:40 +0200","From":"Jiri Pirko ","To":"Yunsheng Lin ","Cc":"\"davem@davemloft.net\" ,\n\thuangdaode ,\n\t\"xuwei (O)\" ,\n\t\"Liguozhu (Kenneth)\" ,\n\t\"Zhuangyuzeng (Yisen)\" ,\n\tGabriele Paoloni ,\n\tJohn Garry , Linuxarm ,\n\tSalil Mehta ,\n\t\"lipeng (Y)\" ,\n\t\"netdev@vger.kernel.org\" ,\n\t\"linux-kernel@vger.kernel.org\" ","Subject":"Re: [PATCH net-next 10/10] net: hns3: Add mqprio support when\n\tinteracting with network stack","Message-ID":"<20170925065740.GB1899@nanopsycho>","References":"<1505992913-107256-1-git-send-email-linyunsheng@huawei.com>\n\t<1505992913-107256-11-git-send-email-linyunsheng@huawei.com>\n\t<20170922125541.GA2005@nanopsycho.orion>\n\t<59c51a37.a1c4df0a.ac4e2.8df0SMTPIN_ADDED_BROKEN@mx.google.com>\n\t<20170922160322.GB2005@nanopsycho.orion>\n\t\n\t<20170924113724.GA2029@nanopsycho>\n\t<290b0679-bfc2-c23c-00ee-43768c1c2327@huawei.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<290b0679-bfc2-c23c-00ee-43768c1c2327@huawei.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1774468,"web_url":"http://patchwork.ozlabs.org/comment/1774468/","msgid":"","list_archive_url":null,"date":"2017-09-25T07:22:43","subject":"Re: [PATCH net-next 10/10] net: hns3: Add mqprio support when\n\tinteracting with network stack","submitter":{"id":71804,"url":"http://patchwork.ozlabs.org/api/people/71804/","name":"Yunsheng Lin","email":"linyunsheng@huawei.com"},"content":"Hi, Jiri\n\nOn 2017/9/25 14:57, Jiri Pirko wrote:\n> Mon, Sep 25, 2017 at 02:45:08AM CEST, linyunsheng@huawei.com wrote:\n>> Hi, Jiri\n>>\n>> On 2017/9/24 19:37, Jiri Pirko wrote:\n>>> Sat, Sep 23, 2017 at 02:47:20AM CEST, linyunsheng@huawei.com wrote:\n>>>> Hi, Jiri\n>>>>\n>>>> On 2017/9/23 0:03, Jiri Pirko wrote:\n>>>>> Fri, Sep 22, 2017 at 04:11:51PM CEST, linyunsheng@huawei.com wrote:\n>>>>>> Hi, Jiri\n>>>>>>\n>>>>>>>> - if (!tc) {\n>>>>>>>> + if (if_running) {\n>>>>>>>> + (void)hns3_nic_net_stop(netdev);\n>>>>>>>> + msleep(100);\n>>>>>>>> + }\n>>>>>>>> +\n>>>>>>>> + ret = (kinfo->dcb_ops && kinfo->dcb_ops->>setup_tc) ?\n>>>>>>>> + kinfo->dcb_ops->setup_tc(h, tc, prio_tc) : ->EOPNOTSUPP;\n>>>>>>\n>>>>>>> This is most odd. Why do you call dcb_ops from >ndo_setup_tc callback?\n>>>>>>> Why are you mixing this together? prio->tc mapping >can be done\n>>>>>>> directly in dcbnl\n>>>>>>\n>>>>>> Here is what we do in dcb_ops->setup_tc:\n>>>>>> Firstly, if current tc num is different from the tc num\n>>>>>> that user provide, then we setup the queues for each\n>>>>>> tc.\n>>>>>>\n>>>>>> Secondly, we tell hardware the pri to tc mapping that\n>>>>>> the stack is using. In rx direction, our hardware need\n>>>>>> that mapping to put different packet into different tc'\n>>>>>> queues according to the priority of the packet, then\n>>>>>> rss decides which specific queue in the tc should the\n>>>>>> packet goto.\n>>>>>>\n>>>>>> By mixing, I suppose you meant why we need the\n>>>>>> pri to tc infomation?\n>>>>>\n>>>>> by mixing, I mean what I wrote. You are calling dcb_ops callback from\n>>>>> ndo_setup_tc callback. So you are mixing DCBNL subsystem and TC\n>>>>> subsystem. Why? Why do you need sch_mqprio? Why DCBNL is not enough for\n>>>>> all?\n>>>>\n>>>> When using lldptool, dcbnl is involved.\n>>>>\n>>>> But when using tc qdisc, dcbbl is not involved, below is the a few key\n>>>> call graph in the kernel when tc qdisc cmd is executed.\n>>>>\n>>>> cmd:\n>>>> tc qdisc add dev eth0 root handle 1:0 mqprio num_tc 4 map 1 2 3 3 1 3 1 1 hw 1\n>>>>\n>>>> call graph:\n>>>> rtnetlink_rcv_msg -> tc_modify_qdisc -> qdisc_create -> mqprio_init ->\n>>>> hns3_nic_setup_tc\n>>>>\n>>>> When hns3_nic_setup_tc is called, we need to know how many tc num and\n>>>> prio_tc mapping from the tc_mqprio_qopt which is provided in the paramter\n>>>> in the ndo_setup_tc function, and dcb_ops is the our hardware specific\n>>>> method to setup the tc related parameter to the hardware, so this is why\n>>>> we call dcb_ops callback in ndo_setup_tc callback.\n>>>>\n>>>> I hope this will answer your question, thanks for your time.\n>>>\n>>> Okay. I understand that you have a usecase for mqprio mapping offload\n>>> without lldptool being involved. Ok. I believe it is wrong to call dcb_ops\n>>> from tc callback. You should have a generic layer inside the driver and\n>>> call it from both dcb_ops and tc callbacks.\n>>\n>> Actually, dcb_ops is our generic layer inside the driver.\n>> Below is high level architecture:\n>>\n>> [ tc qdisc ]\t [ lldpad ]\n>> | |\n>> | |\n>> | |\n>> [ hns3_enet ] [ hns3_dcbnl ]\n>> \\ /\n>> \\ /\n>> \\ /\n>> [ hclge_dcb ]\n>> / \\\n>> / \\\n>> / \\\n>> [ hclgc_main ] [ hclge_tm ]\n>>\n>> hns3_enet.c implements the ndo_setup_tc callback.\n>> hns3_dcbnl.c implements the dcbnl_rtnl_ops for stack's DCBNL system.\n>> hclge_dcb implements the dcb_ops.\n>> So we already have a generic layer that tc and dcbnl all call from.\n>>\n>>>\n>>> Also, what happens If I run lldptool concurrently with mqprio? Who wins\n>>> and is going to configure the mapping?\n>>\n>> Both lldptool and tc qdisc cmd use rtnl interface provided by stack, so\n>> they are both protected by rtnl_lock, so we do not have to do the locking\n>> in the driver.\n> \n> I was not asking about locking, which is obvious, I was asking about the\n> behaviour. Like for example:\n> If I use tc to configure some mapping, later on I run lldptool and change\n> the mapping. Does the tc dump show the updated values or the original\n> ones?\n\nIf it is that case, tc dump show the updated values.\nNormally, we use tc qdisc to configure the netdev to use mqprio, and\nthen use the lldptool the tc_prio mapping, tc schedule mode, tc bandwidth\nand pfc option.\n\nif lldptool change the tc num and tc_prio mapping, it will tell the tc\nsystem by the following function which is called in client_ops->setup_tc:\nnetdev_set_tc_queue\nnetdev_set_prio_tc_map\n\nSo lldptool and tc qdisc can work together.\n\n\n\n> \n>>\n>> The locking is in rtnetlink_rcv_msg:\n>>\n>> \trtnl_lock();\n>> \thandlers = rtnl_dereference(rtnl_msg_handlers[family]);\n>> \tif (handlers) {\n>> \t\tdoit = READ_ONCE(handlers[type].doit);\n>> \t\tif (doit)\n>> \t\t\terr = doit(skb, nlh, extack);\n>> \t}\n>> \trtnl_unlock();\n>>\n>> Thanks.\n>>\n>>>\n>>>\n>>>>\n>>>>>\n>>>>>\n>>>>>\n>>>>>> I hope I did not misunderstand your question, thanks\n>>>>>> for your time reviewing.\n>>>>>\n>>>>> .\n>>>>>\n>>>>\n>>>\n>>> .\n>>>\n>>\n> \n> .\n>","headers":{"Return-Path":"","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=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y0wXq6GZCz9tXM\n\tfor ;\n\tMon, 25 Sep 2017 17:23:07 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S933410AbdIYHXA (ORCPT );\n\tMon, 25 Sep 2017 03:23:00 -0400","from szxga05-in.huawei.com ([45.249.212.191]:6568 \"EHLO\n\tszxga05-in.huawei.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S932578AbdIYHW7 (ORCPT\n\t); Mon, 25 Sep 2017 03:22:59 -0400","from 172.30.72.60 (EHLO DGGEMS413-HUB.china.huawei.com)\n\t([172.30.72.60])\n\tby dggrg05-dlp.huawei.com (MOS 4.4.6-GA FastPath queued)\n\twith ESMTP id DHZ22050; Mon, 25 Sep 2017 15:22:51 +0800 (CST)","from [127.0.0.1] (10.74.191.121) by DGGEMS413-HUB.china.huawei.com\n\t(10.3.19.213) with Microsoft SMTP Server id 14.3.301.0;\n\tMon, 25 Sep 2017 15:22:43 +0800"],"Subject":"Re: [PATCH net-next 10/10] net: hns3: Add mqprio support when\n\tinteracting with network stack","To":"Jiri Pirko ","CC":"\"davem@davemloft.net\" ,\n\thuangdaode ,\n\t\"xuwei (O)\" ,\n\t\"Liguozhu (Kenneth)\" ,\n\t\"Zhuangyuzeng (Yisen)\" ,\n\tGabriele Paoloni ,\n\tJohn Garry , Linuxarm ,\n\t\"Salil Mehta\" ,\n\t\"lipeng (Y)\" ,\n\t\"netdev@vger.kernel.org\" ,\n\t\"linux-kernel@vger.kernel.org\" ","References":"<1505992913-107256-1-git-send-email-linyunsheng@huawei.com>\n\t<1505992913-107256-11-git-send-email-linyunsheng@huawei.com>\n\t<20170922125541.GA2005@nanopsycho.orion>\n\t<59c51a37.a1c4df0a.ac4e2.8df0SMTPIN_ADDED_BROKEN@mx.google.com>\n\t<20170922160322.GB2005@nanopsycho.orion>\n\t\n\t<20170924113724.GA2029@nanopsycho>\n\t<290b0679-bfc2-c23c-00ee-43768c1c2327@huawei.com>\n\t<20170925065740.GB1899@nanopsycho>","From":"Yunsheng Lin ","Message-ID":"","Date":"Mon, 25 Sep 2017 15:22:43 +0800","User-Agent":"Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.0","MIME-Version":"1.0","In-Reply-To":"<20170925065740.GB1899@nanopsycho>","Content-Type":"text/plain; charset=\"utf-8\"","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[10.74.191.121]","X-CFilter-Loop":"Reflected","X-Mirapoint-Virus-RAPID-Raw":"score=unknown(0),\n\trefid=str=0001.0A090204.59C8AECB.00F8, ss=1, re=0.000, recu=0.000,\n\treip=0.000, cl=1, cld=1, fgs=0, ip=0.0.0.0,\n\tso=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32","X-Mirapoint-Loop-Id":"7a8eac6dfc757f8856b750674479f950","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]