From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 1D308A04A5;
Tue, 25 Jan 2022 19:53:26 +0100 (CET)
Received: from [217.70.189.124] (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id 99C644117D;
Tue, 25 Jan 2022 19:53:25 +0100 (CET)
Received: from mail-il1-f172.google.com (mail-il1-f172.google.com
[209.85.166.172])
by mails.dpdk.org (Postfix) with ESMTP id B8EC941161
for <dev@dpdk.org>; Tue, 25 Jan 2022 19:53:23 +0100 (CET)
Received: by mail-il1-f172.google.com with SMTP id d3so17636442ilr.10
for <dev@dpdk.org>; Tue, 25 Jan 2022 10:53:23 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;
h=mime-version:references:in-reply-to:from:date:message-id:subject:to
:cc; bh=9AeW9W+xT/qaAiwUstLvcFLGjrn/CJ4gjS5vWg7rirs=;
b=F5UB0pGl6U29QMAhBrmtN81uvYx0WJQiWHwPZ5Ha8RLxlKoyNcFyzXrcengSSZoucG
CNQL+CftIwTVFEVmdi46EOsVTzj6DmZPc7ZgIbjt0ta+GrAoEo8fP2ft1DUEGb4zuxxO
vnXj27Fwx0Bf0uHxxfc1+wdu5s39ZRJ8kI7Dc0Rd0BAjbOptkGzzyCA2L9NR6jUuTD9v
pQuuUBUDb7kfbOcyeNVd2dPeDJp2i/AYA112BOzfATPqM4pXkBvLBko02Egk7b5hPW38
Ao2UbtsWZy3Ro3kuTtn+/NyIyJ88bnpsgbq+s56MZnnEKkPB7gTyHIUHh2btk26EWKYq
KP4Q==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20210112;
h=x-gm-message-state:mime-version:references:in-reply-to:from:date
:message-id:subject:to:cc;
bh=9AeW9W+xT/qaAiwUstLvcFLGjrn/CJ4gjS5vWg7rirs=;
b=dqYF/5OCDBx1d0PgMM4tEtmyvL0398G9oCIQ99QiHst93IOcMkUOKUig0EGTwC+mls
hwQWhYepLYey/dhvasjn34c3iIm/LY5DfhLia7V5ldm2L8wkZKMoYv2AZhLR01TD4IyX
bji5Xm0PlTa3gn3rJJRhPE3BbgO+dVX1v9zO9QkXZLqKr0MW+/fECK0vONYMpMMnZ7SR
Fh3APuOYxHdTpO2AC4srJ/4NHzpfBxYrfnQQp3FpMiKsFJYP+UHzAv0iKMJuCnpp+HNW
o6sMo1D5QwaShZo5TEfOZLK9a5u7W4CIqgFprcM7DhUoyElRx9NUdlf/F7/trVl4Wk9y
hflg==
X-Gm-Message-State: AOAM531zuEsS396wpG0XXCoYR+Wjc184zGDCGrA4cdb1y0I0DNNe2kJn
N58pOsPgwRelEsCEVdh9isazC672D4ZI8rtMjsc=
X-Google-Smtp-Source: ABdhPJzel9/7c7y4gsoPquvnTOkmSyxb5+Xycf2ZegrKFj3WMcj2M/1g3IEfzT8MHap0L2eTt/v3/dLs3ummGRN+uwU=
X-Received: by 2002:a05:6e02:1809:: with SMTP id
a9mr12699989ilv.318.1643136802977;
Tue, 25 Jan 2022 10:53:22 -0800 (PST)
MIME-Version: 1.0
References: <20220109105851.734687-1-skori@marvell.com>
<20220113102718.3167282-1-jerinj@marvell.com>
<000cbfe1-9d91-3011-4cf9-56dbeebcebb6@intel.com>
In-Reply-To: <000cbfe1-9d91-3011-4cf9-56dbeebcebb6@intel.com>
From: Jerin Jacob <jerinjacobk@gmail.com>
Date: Wed, 26 Jan 2022 00:22:56 +0530
Message-ID: <CALBAE1P-QVQ1HrfAxS3irjb3kXRfs7Y1F5w4ptyy_4WwPz_gVg@mail.gmail.com>
Subject: Re: [dpdk-dev] [PATCH v2 1/2] ethdev: support queue-based priority
flow control
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: Jerin Jacob <jerinj@marvell.com>, dpdk-dev <dev@dpdk.org>,
Ray Kinsella <mdr@ashroe.eu>, Thomas Monjalon <thomas@monjalon.net>,
Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
Ajit Khaparde <ajit.khaparde@broadcom.com>, Andrew Boyer <aboyer@pensando.io>,
Beilei Xing <beilei.xing@intel.com>, "Richardson,
Bruce" <bruce.richardson@intel.com>,
Chas Williams <chas3@att.com>, "Xia, Chenbo" <chenbo.xia@intel.com>,
Ciara Loftus <ciara.loftus@intel.com>,
Devendra Singh Rawat <dsinghrawat@marvell.com>,
Ed Czeck <ed.czeck@atomicrules.com>, Evgeny Schemeilin <evgenys@amazon.com>,
Gaetan Rivet <grive@u256.net>, Gagandeep Singh <g.singh@nxp.com>,
Guoyang Zhou <zhouguoyang@huawei.com>,
Haiyue Wang <haiyue.wang@intel.com>, Harman Kalra <hkalra@marvell.com>,
heinrich.kuhn@corigine.com,
Hemant Agrawal <hemant.agrawal@nxp.com>, Hyong Youb Kim <hyonkim@cisco.com>,
Igor Chauskin <igorch@amazon.com>, Igor Russkikh <irusskikh@marvell.com>,
Jakub Grajciar <jgrajcia@cisco.com>,
Jasvinder Singh <jasvinder.singh@intel.com>,
Jian Wang <jianwang@trustnetic.com>, Jiawen Wu <jiawenwu@trustnetic.com>,
Jingjing Wu <jingjing.wu@intel.com>, John Daley <johndale@cisco.com>,
John Miller <john.miller@atomicrules.com>,
"John W. Linville" <linville@tuxdriver.com>,
"Wiles, Keith" <keith.wiles@intel.com>, Kiran Kumar K <kirankumark@marvell.com>,
Lijun Ou <oulijun@huawei.com>, Liron Himi <lironh@marvell.com>,
Long Li <longli@microsoft.com>,
Marcin Wojtas <mw@semihalf.com>, Martin Spinler <spinler@cesnet.cz>,
Matan Azrad <matan@nvidia.com>, Matt Peters <matt.peters@windriver.com>,
Maxime Coquelin <maxime.coquelin@redhat.com>,
Michal Krawczyk <mk@semihalf.com>, "Min Hu (Connor" <humin29@huawei.com>,
Pradeep Kumar Nalla <pnalla@marvell.com>,
Nithin Dabilpuram <ndabilpuram@marvell.com>,
Qiming Yang <qiming.yang@intel.com>, Qi Zhang <qi.z.zhang@intel.com>,
Radha Mohan Chintakuntla <radhac@marvell.com>,
Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>,
Rasesh Mody <rmody@marvell.com>, Rosen Xu <rosen.xu@intel.com>,
Sachin Saxena <sachin.saxena@oss.nxp.com>,
Satha Koteswara Rao Kottidi <skoteshwar@marvell.com>,
Shahed Shaikh <shshaikh@marvell.com>, Shai Brandes <shaibran@amazon.com>,
Shepard Siegel <shepard.siegel@atomicrules.com>,
Somalapuram Amaranath <asomalap@amd.com>,
Somnath Kotur <somnath.kotur@broadcom.com>,
Stephen Hemminger <sthemmin@microsoft.com>,
Steven Webster <steven.webster@windriver.com>,
Sunil Kumar Kori <skori@marvell.com>, Tetsuya Mukawa <mtetsuyah@gmail.com>,
Veerasenareddy Burru <vburru@marvell.com>,
Viacheslav Ovsiienko <viacheslavo@nvidia.com>,
Xiao Wang <xiao.w.wang@intel.com>, Xiaoyun Wang <cloud.wangxiaoyun@huawei.com>,
Yisen Zhuang <yisen.zhuang@huawei.com>, Yong Wang <yongwang@vmware.com>,
Ziyang Xuan <xuanziyang2@huawei.com>
Content-Type: text/plain; charset="UTF-8"
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
On Tue, Jan 25, 2022 at 11:05 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 1/13/2022 10:27 AM, jerinj@marvell.com wrote:
> > From: Jerin Jacob <jerinj@marvell.com>
> >
> > Based on device support and use-case need, there are two different ways
> > to enable PFC. The first case is the port level PFC configuration, in
> > this case, rte_eth_dev_priority_flow_ctrl_set() API shall be used to
> > configure the PFC, and PFC frames will be generated using based on VLAN
> > TC value.
> >
> > The second case is the queue level PFC configuration, in this
> > case, Any packet field content can be used to steer the packet to the
> > specific queue using rte_flow or RSS and then use
> > rte_eth_dev_priority_flow_ctrl_queue_set() to set the TC mapping on each
> > queue. Based on congestion selected on the specific queue, configured TC
> > shall be used to generate PFC frames.
> > > Operation of these modes are mutually exclusive, when driver sets
> > non zero value for rte_eth_dev_info::pfc_queue_tc_max,
> > application must use queue level PFC configuration via
> > rte_eth_dev_priority_flow_ctrl_queue_set() API instead of port level
> > PFC configuration via rte_eth_dev_priority_flow_ctrl_set() API to
> > realize PFC configuration.
> >
> > This patch enables the configuration for second case a.k.a queue
> > based PFC also updates rte_eth_dev_priority_flow_ctrl_set()
> > implmentaion to adheher to rte_eth_dev_info::pfc_queue_tc_max
> > handling.
> >
> > Also updated libabigail.abignore to ignore the update
> > to reserved fields in rte_eth_dev_info.
> >
> > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> > ---
> >
> > A driver implemtion based on this API is at
> > https://patches.dpdk.org/project/dpdk/patch/20220111081831.881374-1-skori@marvell.com/
> >
> > RFC..v1
> > - Added queue based PFC config API instead port based
> >
> > v1..v2
> > +
> > /**
> > * Tunnel type for device-specific classifier configuration.
> > * @see rte_eth_udp_tunnel
> > @@ -1841,8 +1863,30 @@ struct rte_eth_dev_info {
> > * embedded managed interconnect/switch.
> > */
> > struct rte_eth_switch_info switch_info;
> > -
> > - uint64_t reserved_64s[2]; /**< Reserved for future fields */
> > + /**
> > + * Maximum supported traffic class as per PFC (802.1Qbb) specification.
> > + *
> > + * Based on device support and use-case need, there are two different
> > + * ways to enable PFC. The first case is the port level PFC
> > + * configuration, in this case, rte_eth_dev_priority_flow_ctrl_set()
> > + * API shall be used to configure the PFC, and PFC frames will be
> > + * generated using based on VLAN TC value.
> > + * The second case is the queue level PFC configuration, in this case,
> > + * Any packet field content can be used to steer the packet to the
> > + * specific queue using rte_flow or RSS and then use
> > + * rte_eth_dev_priority_flow_ctrl_queue_set() to set the TC mapping
> > + * on each queue. Based on congestion selected on the specific queue,
> > + * configured TC shall be used to generate PFC frames.
> > + *
> > + * When set to non zero value, application must use queue level
> > + * PFC configuration via rte_eth_dev_priority_flow_ctrl_queue_set() API
> > + * instead of port level PFC configuration via
> > + * rte_eth_dev_priority_flow_ctrl_set() API to realize
> > + * PFC configuration.
> > + */
> > + uint8_t pfc_queue_tc_max;
>
>
> 'rte_eth_dev_info_get()' is one of the APIs that anyone using ethdev needs to use.
>
> Instead of expanding it with less used features, what do you think to have a
> specific API to get the 'pfc_queue_tc_max'?
OK. rte_eth_dev_info_get() was general theme used in ethdev API. Fine
for new API
specifically for this.
>
> It also can be used by application to detect queue based PFC is supported by
> driver or not.
OK.
> Assume API is 'rte_eth_dev_priority_flow_ctrl_get()', if it returns '-ENOTSUP'
> application can know that PMD doesn't support queue based PFC.
OK.
>
>
> > + uint8_t reserved_8s[7];
> > + uint64_t reserved_64s[1]; /**< Reserved for future fields */
> > void *reserved_ptrs[2]; /**< Reserved for future fields */
> > };
> >
> > @@ -4109,6 +4153,9 @@ int rte_eth_dev_flow_ctrl_set(uint16_t port_id,
> > * Configure the Ethernet priority flow control under DCB environment
> > * for Ethernet device.
> > *
> > + * @see struct rte_eth_dev_info::pfc_queue_tc_max priority
> > + * flow control usage models.
> > + *
> > * @param port_id
> > * The port identifier of the Ethernet device.
> > * @param pfc_conf
> > @@ -4119,10 +4166,40 @@ int rte_eth_dev_flow_ctrl_set(uint16_t port_id,
> > * - (-ENODEV) if *port_id* invalid.
> > * - (-EINVAL) if bad parameter
> > * - (-EIO) if flow control setup failure or device is removed.
> > + *
> > */
> > int rte_eth_dev_priority_flow_ctrl_set(uint16_t port_id,
> > - struct rte_eth_pfc_conf *pfc_conf);
> > + struct rte_eth_pfc_conf *pfc_conf);
>
> Above syntax changes are not needed.
>
> DPDK coding convention is using tabs (mostly two) for multi line function
> decleration/definition, please be consistant with usage, this patch has
> multiple variations.
Ack.
>
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Configure the Ethernet priority flow control for a given queue
> > + * for Ethernet device.
> > + *
> > + * @see struct rte_eth_dev_info::pfc_queue_tc_max priority flow control
> > + * usage models.
> > + *
> > + * @note When an ethdev port switches to PFC mode, the unconfigured
>
> Doesit mean queue based PFC mode?
Yes. I will change to PFC mode -> queue-based PFC mode.
>
> > + * queues shall be configured by the driver with default values such as
> > + * lower priority value for TC etc.
> > + *
>
> I assume there is no way for application to know what the defaults values are,
> also not sure if application interested in this.
Yes. I don't think the application cares. But added in the doc to
clarify the state.
>
> > + * @param port_id
> > + * The port identifier of the Ethernet device.
> > + * @param pfc_queue_conf
> > + * The pointer to the structure of the priority flow control parameters
> > + * for the queue.
> > + * @return
> > + * - (0) if successful.
> > + * - (-ENOTSUP) if hardware doesn't support priority flow control mode.
> > + * - (-ENODEV) if *port_id* invalid.
> > + * - (-EINVAL) if bad parameter
> > + * - (-EIO) if flow control setup queue failure
> > + */
> > +__rte_experimental
> > +int rte_eth_dev_priority_flow_ctrl_queue_set(uint16_t port_id,
> > + struct rte_eth_pfc_queue_conf *pfc_queue_conf);
>
> I wonder if Rx/Tx queue id should be API arguments, to be consistent
> with some other APIs, and will it help application that configures queues
> in a loop.
> But I can see 'rx_pause' or 'tx_pause' (in config) can be valid or not
> based on the 'mode', so I understand to have queue ids in the struct.
> No strong opinion.
Yes. I keep it as is.