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 694FFA0C4C; Tue, 23 Nov 2021 11:58:46 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EC83A40040; Tue, 23 Nov 2021 11:58:45 +0100 (CET) Received: from mail-il1-f170.google.com (mail-il1-f170.google.com [209.85.166.170]) by mails.dpdk.org (Postfix) with ESMTP id 769544003C for ; Tue, 23 Nov 2021 11:58:45 +0100 (CET) Received: by mail-il1-f170.google.com with SMTP id x9so21225620ilu.6 for ; Tue, 23 Nov 2021 02:58:45 -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=pKfKzfRX0ALM9BRPfDadLon/RUxDQkCU5AyfysAcM3w=; b=KKxUQMLnyFVaybi2IKhfOP6Zlpazg4qwq6pkkQhNzUzLM1KcA3cURWQ27dOEe0RgIv qsiAZp6g0jz0DDiCT6nydD3xHdOTVR5wHkLJLDYsinJvbm7spX5XbasHOO7AnpJWrkUn W6YdL/7B+eqfXonSl72cd+8I3slAsJpyBM2ng3Mse+YIPlTcAIWW21g+FDd/5ycHNSzc QIeLCLQz/UIHfqPjaYXRkPMyp4UHHg0kc/enFv7rCAKBhKORq6geCAiaCNKgKtgHmT2N HLEpmJzTWnF+4FxrHqTbl12d5PALhR1PVBhjJmgq1Id374TtKJGi9Nl5qiOCbJmv2RPa hk3Q== 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=pKfKzfRX0ALM9BRPfDadLon/RUxDQkCU5AyfysAcM3w=; b=C9G8xkpCjRSFxFZ7BY0ELHNaKkGqxJiBDoeJ/RaAPD9eYADIqdN+9Gru5ldXQqXKZ+ G5dkN27MojU+/9iGHMEWAle98ky9ComhL4AHAoEbwR8Pt7Uvfl7Un1vXsWqCNjTCnQFC Zk3AxpSc3q+XbABM0qDbOVBe9wOr+VNDOglXnlY9jyqlYmeiIyCA+Rj+/KZQQ+VcDTQy UcSNroUinthsZbM9fn1LBQSxq4QYHSutcCdASLJowFOhC+h7MYSUyIte1pMw+XROuiU3 7pzgBqLde5I4N6Hk22CvA592yatWsxtTgvs613rZDsz/WfJjKsWSxps+mEiEN/3+fAFX S9qg== X-Gm-Message-State: AOAM531a1v5Bj8zGBFnBdCEHL+A9aKcwf7wWqEYENftp7UG1GCHUpagH TKssvrpBtNBmZ1CLbQioQ1dx3Tu/QaPoQHvAfRA= X-Google-Smtp-Source: ABdhPJxUsFAC6WLKvUehiF5LW+mi/UA+F4v4mwHWDrXk5ywjRXXYB/Iw4XOo10FoETkV+9/roC4ZTHSGy4EElusLZ54= X-Received: by 2002:a05:6e02:12e2:: with SMTP id l2mr4878304iln.251.1637665124774; Tue, 23 Nov 2021 02:58:44 -0800 (PST) MIME-Version: 1.0 References: <20211005125923.2651449-1-jerinj@marvell.com> In-Reply-To: From: Jerin Jacob Date: Tue, 23 Nov 2021 16:28:18 +0530 Message-ID: Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control To: Ori Kam Cc: Jerin Jacob , dpdk-dev , "NBU-Contact-Thomas Monjalon (EXTERNAL)" , 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 , "NBU-Contact-longli (EXTERNAL)" , 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 , Slava 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, Nov 21, 2021 at 3:20 PM Ori Kam wrote: > > Hi Jerin, Hi Ori, > > Sorry for my late response, Thanks for the review. > > > -----Original Message----- > > From: Jerin Jacob > > Sent: Wednesday, November 17, 2021 11:49 AM > > To: Jerin Jacob > > Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: support priority based flow control > > > > On Tue, Oct 5, 2021 at 6:32 PM wrote: > > > > > > From: Jerin Jacob > > > > > > rte_eth_dev_priority_flow_ctrl_set() based API is not generic as it > > > can not support other than VLAN priority mapping to PFC traffic class. > > > > > > Introducing RTE_FLOW_ACTION_TYPE_PFC_SET_TC rte_flow action to > > > set the traffic class as per 802.1Qbb specification. This will enable, > > > Traffic class(8bit) to be selected based on any packet field like DSCP. > > > > > > Also, making it as rte_flow action will enable fine control on > > > traffic class selection to a specific queue or VF etc. > > > > > > Signed-off-by: Jerin Jacob > > > > Ping. If there are no comments on RFC, Planning to send v1 for 22.02. > > All the set type of functions are going to be deprecated. > you should use RTE_FLOW_ACTION_TYPE_MODIFY_FIELD. > > What is the item that you are matching on when using rte_flow? Is it part of the tci in the vlan item? TC can be VLAN TCI field or DSCP field in IP header or any other field in packet. We need to set the traffic class as per 802.1Qbb specification, May I know how the "modify" attribute helps here. It should be a "set" operation. Right? > > > > > > > --- > > > > > > Planning to submit the testpmd and cnxk ethdev driver changes after receiving > > > the feedback on this. > > > > > > > > > doc/guides/prog_guide/rte_flow.rst | 24 ++++++++++++++++++++++++ > > > lib/ethdev/rte_flow.c | 1 + > > > lib/ethdev/rte_flow.h | 27 +++++++++++++++++++++++++++ > > > 3 files changed, 52 insertions(+) > > > > > > diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst > > > index 2b42d5ec8c..e59f8a2902 100644 > > > --- a/doc/guides/prog_guide/rte_flow.rst > > > +++ b/doc/guides/prog_guide/rte_flow.rst > > > @@ -2999,6 +2999,30 @@ which is set in the packet meta-data (i.e. struct ``rte_mbuf::sched::color``) > > > | ``meter_color`` | Packet color | > > > +-----------------+--------------+ > > > > > > +Action: ``PFC_SET_TC`` > > > +^^^^^^^^^^^^^^^^^^^^^^ > > > + > > > +Set traffic class as per PFC (802.1Qbb) specification. > > > + > > > +This action must be used with any of the following action. > > > + > > > +- ``RTE_FLOW_ACTION_TYPE_QUEUE`` > > > +- ``RTE_FLOW_ACTION_TYPE_RSS`` > > > +- ``RTE_FLOW_ACTION_TYPE_PF`` > > > +- ``RTE_FLOW_ACTION_TYPE_VF`` > > > +- ``RTE_FLOW_ACTION_TYPE_PHY_PORT`` > > > +- ``RTE_FLOW_ACTION_TYPE_PORT_ID`` > > > + > > Why? All the above actions are terminating actions so if I want ot match on the value > it doesn't make sense to have it only on the last rule. In PFC, we are specifying, Given TC needs to steer to specific Queue, RSS, PF etc. Not sure how other actions are relevant for SET_TC action. Do you have any specific action in mind where SET_TC valid in addition to above actions > > > > +.. _table_rte_flow_action_pfc_set_tc: > > > + > > > +.. table:: PFC_SET_PRIORITY > > > + > > > + +-----------------+-------------------------------------+ > > > + | Field | Value | > > > + +=================+=====================================+ > > > + | ``tc`` | Traffic class as per PFC (802.1Qbb) | > > > + +-----------------+-------------------------------------+ > > > + > > > Negative types > > > ~~~~~~~~~~~~~~ > > > > > > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c > > > index 8cb7a069c8..75c661159e 100644 > > > --- a/lib/ethdev/rte_flow.c > > > +++ b/lib/ethdev/rte_flow.c > > > @@ -189,6 +189,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[] = { > > > */ > > > MK_FLOW_ACTION(INDIRECT, 0), > > > MK_FLOW_ACTION(CONNTRACK, sizeof(struct rte_flow_action_conntrack)), > > > + MK_FLOW_ACTION(PFC_SET_TC, sizeof(struct rte_flow_action_pfc_set_tc)), > > > }; > > > > > > int > > > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h > > > index 7b1ed7f110..5298418e9e 100644 > > > --- a/lib/ethdev/rte_flow.h > > > +++ b/lib/ethdev/rte_flow.h > > > @@ -2409,6 +2409,13 @@ enum rte_flow_action_type { > > > * See struct rte_flow_action_meter_color. > > > */ > > > RTE_FLOW_ACTION_TYPE_METER_COLOR, > > > + > > > + /** > > > + * Set traffic class as per PFC (802.1Qbb) specification. > > > + * > > > + * See struct rte_flow_action_pfc_set_tc. > > > + */ > > > + RTE_FLOW_ACTION_TYPE_PFC_SET_TC, > > > }; > > > > > > /** > > > @@ -3168,6 +3175,26 @@ struct rte_flow_action_meter_color { > > > enum rte_color color; /**< Packet color. */ > > > }; > > > > > > +/** > > > + * @warning > > > + * @b EXPERIMENTAL: this structure may change without prior notice > > > + * > > > + * RTE_FLOW_ACTION_TYPE_PFC_SET_TC > > > + * > > > + * Set traffic class as per PFC (802.1Qbb) specification. > > > + * > > > + * This action must be used any of the following action. > > > + * - RTE_FLOW_ACTION_TYPE_QUEUE, > > > + * - RTE_FLOW_ACTION_TYPE_RSS, > > > + * - RTE_FLOW_ACTION_TYPE_PF, > > > + * - RTE_FLOW_ACTION_TYPE_VF, > > > + * - RTE_FLOW_ACTION_TYPE_PHY_PORT, > > > + * - RTE_FLOW_ACTION_TYPE_PORT_ID > > > + */ > > What does it mean? I must use it only on rules that have one of the above actions? See above. > > > > +struct rte_flow_action_pfc_set_tc { > > > + uint8_t tc; /**< Traffic class as per PFC (802.1Qbb) specification */ > > > +}; > > > + > > > /** > > > * Field IDs for MODIFY_FIELD action. > > > */ > > > -- > > > 2.33.0 > > > > > Best, > Ori