From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 64CDBA034F; Mon, 6 Dec 2021 10:57:35 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4A55E4013F; Mon, 6 Dec 2021 10:57:35 +0100 (CET) Received: from mail-il1-f175.google.com (mail-il1-f175.google.com [209.85.166.175]) by mails.dpdk.org (Postfix) with ESMTP id 4CD5640040 for ; Mon, 6 Dec 2021 10:57:34 +0100 (CET) Received: by mail-il1-f175.google.com with SMTP id t8so9585940ilu.8 for ; Mon, 06 Dec 2021 01:57:34 -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=mLmj7Dhi7zWGKvLzZCOdsEuJYIAh/GQNH0Z33+P51yw=; b=XwiNMjUOWvhiAqFdS1sxzOafpDMsy0VLZXUnhO32KrB+I8zWvAIj0CaWgEiOiYzRf/ fIbClD/5WBcq00uYVxZy8KzUQWCUbkNK/+PfD5bY9xQlyUfHdHAjtaaMne91zfb7j6pE uDk00sqa72xigtfJCITL/Xq3GHit18qEXNfuoDBBG2xpRMfeACal5LCZIDxjmwE3zxG2 AqeEmOnp4Oi1OHFt3mdQCj/7H64zgf44nnXjP1x+CKJKyM7viWr6fREc1Sx2CIA4glKY oC49YjjcRXzD/Avp3sjJESbs6TMN3OJcSJyeoZyYa/y3cThk5/2BC6QxUwv+neSlIWZR lHng== 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=mLmj7Dhi7zWGKvLzZCOdsEuJYIAh/GQNH0Z33+P51yw=; b=KpJm8mHKzjyRORcZ02NsBL+Bhk5sB8TyuOoHaw26XPLk58CI+49aKeiQXfFrZ3PdNK 9LMkp1ldAbh4Bvwi54G10L30DRTg5ZGJoeZd2YOGq7PD6f8R7rotRF5d0c/ml0RvckHN 0ykkhnK9ci0bpAlt9j1w8UQI33ad5eaE0SAZAtuEsM2/IWLnjOVhxgdlJj/ILMPHAmaV sifgdBAXVWcVE3EoS5dCsVajA+Qxgm17ALtXK9T6kLOitGfESTswjCDQ34yvUZeYyYTh C+4K3Wh8HLiUkY9PA4+JOYav+Gr7yJY/4WGXgzasuxv/KDf5J8/JmXIU2FjrPc7+dtpK OOTw== X-Gm-Message-State: AOAM531OblkG36LF742b7ppWkhYKoU3FwdvUUd2KFrjDRTVzro+Ta7Zq PwbSrJ9lC0mGHFVJkss05F4JO4pDEkClzhCF1Is= X-Google-Smtp-Source: ABdhPJyalkT1gmIerqhOU3FShyrxYtYAJvTo6fQ9JFq1QEwPmSbR8I1M8MuxterAtbZTFuow4mVQ17EbHZYf08f05oo= X-Received: by 2002:a05:6e02:1583:: with SMTP id m3mr31783117ilu.294.1638784653573; Mon, 06 Dec 2021 01:57:33 -0800 (PST) MIME-Version: 1.0 References: <20211204172458.1904300-1-jerinj@marvell.com> <20211204093835.4ef4219c@hermes.local> <20211205100031.7be8e7b0@hermes.local> In-Reply-To: <20211205100031.7be8e7b0@hermes.local> From: Jerin Jacob Date: Mon, 6 Dec 2021 15:27:07 +0530 Message-ID: Subject: Re: [dpdk-dev] [PATCH] ethdev: support queue-based priority flow control To: Stephen Hemminger Cc: Jerin Jacob , dpdk-dev , Ray Kinsella , Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko , Ajit Khaparde , Andrew Boyer , Beilei Xing , "Richardson, Bruce" , Chas Williams , "Xia, Chenbo" , Ciara Loftus , Devendra Singh Rawat , Ed Czeck , Evgeny Schemeilin , Gaetan Rivet , Gagandeep Singh , Guoyang Zhou , Haiyue Wang , Harman Kalra , heinrich.kuhn@corigine.com, Hemant Agrawal , Hyong Youb Kim , Igor Chauskin , Igor Russkikh , Jakub Grajciar , Jasvinder Singh , Jian Wang , Jiawen Wu , Jingjing Wu , John Daley , John Miller , "John W. Linville" , "Wiles, Keith" , Kiran Kumar K , Lijun Ou , Liron Himi , Long Li , Marcin Wojtas , Martin Spinler , Matan Azrad , Matt Peters , Maxime Coquelin , Michal Krawczyk , "Min Hu (Connor" , Pradeep Kumar Nalla , Nithin Dabilpuram , Qiming Yang , Qi Zhang , Radha Mohan Chintakuntla , Rahul Lakkireddy , Rasesh Mody , Rosen Xu , Sachin Saxena , Satha Koteswara Rao Kottidi , Shahed Shaikh , Shai Brandes , Shepard Siegel , Somalapuram Amaranath , Somnath Kotur , Stephen Hemminger , Steven Webster , Sunil Kumar Kori , Tetsuya Mukawa , Veerasenareddy Burru , Viacheslav Ovsiienko , Xiao Wang , Xiaoyun Wang , Yisen Zhuang , Yong Wang , Ziyang Xuan 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Sun, Dec 5, 2021 at 11:30 PM Stephen Hemminger wrote: > > On Sun, 5 Dec 2021 12:33:57 +0530 > Jerin Jacob wrote: > > > On Sat, Dec 4, 2021 at 11:08 PM Stephen Hemminger > > wrote: > > > > > > On Sat, 4 Dec 2021 22:54:58 +0530 > > > wrote: > > > > > > > + /** > > > > + * 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; > > > > + uint8_t reserved_8s[7]; > > > > + uint64_t reserved_64s[1]; /**< Reserved for future fields */ > > > > void *reserved_ptrs[2]; /**< Reserved for future fields */ > > > > > > Not sure you can claim ABI compatibility because the previous versions of DPDK > > > did not enforce that reserved fields must be zero. The Linux kernel > > > learned this when adding flags for new system calls; reserved fields only > > > work if you enforce that application must set them to zero. > > > > In this case it rte_eth_dev_info is an out parameter and implementation of > > rte_eth_dev_info_get() already memseting to 0. > > Do you still see any other ABI issue? > > > > See rte_eth_dev_info_get() > > /* > > * Init dev_info before port_id check since caller does not have > > * return status and does not know if get is successful or not. > > */ > > memset(dev_info, 0, sizeof(struct rte_eth_dev_info)); > > The concern was from the misreading comment. It talks about what application should do. > Could you reword the comment so that it describes what pfc_queue_tc_max is here The comment is at rte_eth_dev_info::pfc_queue_tc_max. So it is implied that get pararamter. current comment --- + * 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. --- Is updating to following help to clarify. If so, I will send v2, if not, Please suggest. --- + * When set to non zero value by the driver, 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. --- > and move the flow control set part of the comment to where the API for that is. The comment is needed for rte_eth_dev_priority_flow_ctrl_set() and rte_eth_dev_priority_flow_ctrl_queue_set(). Instead of duplicating the comments, I added the comment at rte_eth_dev_info::pfc_queue_tc_max and added "@see struct rte_eth_dev_info::pfc_queue_tc_max priority flow control usage models" in rte_eth_dev_priority_flow_ctrl_set() and rte_eth_dev_priority_flow_ctrl_queue_set().