RE: [EXT] Re: [dpdk-dev] [PATCH v2 2/2] app/testpmd: add queue based pfc CLI options

[Sunil Kumar Kori <skori@marvell.com>]

-----Original Message-----
From: Ajit Khaparde <ajit.khaparde@broadcom.com> 
Sent: Thursday, January 27, 2022 10:27 PM
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: Sunil Kumar Kori <skori@marvell.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; dev@dpdk.org; Xiaoyun Li <xiaoyun.li@intel.com>; Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>; thomas@monjalon.net; aboyer@pensando.io; andrew.rybchenko@oktetlabs.ru; beilei.xing@intel.com; bruce.richardson@intel.com; chas3@att.com; chenbo.xia@intel.com; ciara.loftus@intel.com; Devendra Singh Rawat <dsinghrawat@marvell.com>; ed.czeck@atomicrules.com; evgenys@amazon.com; grive@u256.net; g.singh@nxp.com; zhouguoyang@huawei.com; haiyue.wang@intel.com; Harman Kalra <hkalra@marvell.com>; heinrich.kuhn@corigine.com; hemant.agrawal@nxp.com; hyonkim@cisco.com; igorch@amazon.com; Igor Russkikh <irusskikh@marvell.com>; jgrajcia@cisco.com; jasvinder.singh@intel.com; jianwang@trustnetic.com; jiawenwu@trustnetic.com; jingjing.wu@intel.com; johndale@cisco.com; john.miller@atomicrules.com; linville@tuxdriver.com; keith.wiles@intel.com; Kiran Kumar Kokkilagadda <kirankumark@marvell.com>; oulijun@huawei.com; Liron Himi <lironh@marvell.com>; longli@microsoft.com; mw@semihalf.com; spinler@cesnet.cz; matan@nvidia.com; matt.peters@windriver.com; maxime.coquelin@redhat.com; mk@semihalf.com; humin29@huawei.com; Pradeep Kumar Nalla <pnalla@marvell.com>; Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>; qiming.yang@intel.com; qi.z.zhang@intel.com; Radha Chintakuntla <radhac@marvell.com>; rahul.lakkireddy@chelsio.com; Rasesh Mody <rmody@marvell.com>; rosen.xu@intel.com; sachin.saxena@oss.nxp.com; Satha Koteswara Rao Kottidi <skoteshwar@marvell.com>; Shahed Shaikh <shshaikh@marvell.com>; shaibran@amazon.com; shepard.siegel@atomicrules.com; asomalap@amd.com; somnath.kotur@broadcom.com; sthemmin@microsoft.com; steven.webster@windriver.com; mtetsuyah@gmail.com; Veerasenareddy Burru <vburru@marvell.com>; viacheslavo@nvidia.com; xiao.w.wang@intel.com; cloud.wangxiaoyun@huawei.com; yisen.zhuang@huawei.com; yongwang@vmware.com; xuanziyang2@huawei.com
Subject: Re: [EXT] Re: [dpdk-dev] [PATCH v2 2/2] app/testpmd: add queue based pfc CLI options

On Thu, Jan 27, 2022 at 2:40 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 1/27/2022 7:13 AM, Sunil Kumar Kori wrote:
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >> Sent: Tuesday, January 25, 2022 11:07 PM
> >> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; dev@dpdk.org; 
> >> Xiaoyun Li <xiaoyun.li@intel.com>; Aman Singh 
> >> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
> >> Cc: thomas@monjalon.net; ajit.khaparde@broadcom.com; 
> >> aboyer@pensando.io; andrew.rybchenko@oktetlabs.ru; 
> >> beilei.xing@intel.com; bruce.richardson@intel.com; chas3@att.com; 
> >> chenbo.xia@intel.com; ciara.loftus@intel.com; Devendra Singh Rawat 
> >> <dsinghrawat@marvell.com>; ed.czeck@atomicrules.com; 
> >> evgenys@amazon.com; grive@u256.net; g.singh@nxp.com; 
> >> zhouguoyang@huawei.com; haiyue.wang@intel.com; Harman Kalra 
> >> <hkalra@marvell.com>; heinrich.kuhn@corigine.com; 
> >> hemant.agrawal@nxp.com; hyonkim@cisco.com; igorch@amazon.com; Igor 
> >> Russkikh <irusskikh@marvell.com>; jgrajcia@cisco.com; 
> >> jasvinder.singh@intel.com; jianwang@trustnetic.com; 
> >> jiawenwu@trustnetic.com; jingjing.wu@intel.com; johndale@cisco.com; 
> >> john.miller@atomicrules.com; linville@tuxdriver.com; 
> >> keith.wiles@intel.com; Kiran Kumar Kokkilagadda 
> >> <kirankumark@marvell.com>; oulijun@huawei.com; Liron Himi 
> >> <lironh@marvell.com>; longli@microsoft.com; mw@semihalf.com; 
> >> spinler@cesnet.cz; matan@nvidia.com; matt.peters@windriver.com; 
> >> maxime.coquelin@redhat.com; mk@semihalf.com; humin29@huawei.com; 
> >> Pradeep Kumar Nalla <pnalla@marvell.com>; Nithin Kumar Dabilpuram 
> >> <ndabilpuram@marvell.com>; qiming.yang@intel.com; 
> >> qi.z.zhang@intel.com; Radha Chintakuntla <radhac@marvell.com>; 
> >> rahul.lakkireddy@chelsio.com; Rasesh Mody <rmody@marvell.com>; 
> >> rosen.xu@intel.com; sachin.saxena@oss.nxp.com; Satha Koteswara Rao 
> >> Kottidi <skoteshwar@marvell.com>; Shahed Shaikh 
> >> <shshaikh@marvell.com>; shaibran@amazon.com; 
> >> shepard.siegel@atomicrules.com; asomalap@amd.com; 
> >> somnath.kotur@broadcom.com; sthemmin@microsoft.com; 
> >> steven.webster@windriver.com; Sunil Kumar Kori <skori@marvell.com>; 
> >> mtetsuyah@gmail.com; Veerasenareddy Burru <vburru@marvell.com>; 
> >> viacheslavo@nvidia.com; xiao.w.wang@intel.com; 
> >> cloud.wangxiaoyun@huawei.com; yisen.zhuang@huawei.com; 
> >> yongwang@vmware.com; xuanziyang2@huawei.com
> >> Subject: [EXT] Re: [dpdk-dev] [PATCH v2 2/2] app/testpmd: add queue 
> >> based pfc CLI options
> >>
> >> External Email
> >>
> >> -------------------------------------------------------------------
> >> --- On 1/13/2022 10:27 AM, jerinj@marvell.com wrote:
> >>> From: Sunil Kumar Kori <skori@marvell.com>
> >>>
> >>> Patch adds command line options to configure queue based priority 
> >>> flow control.
> >>>
> >>> - Syntax command is given as below:
> >>>
> >>> set pfc_queue_ctrl <port_id> rx <on|off> <tx_qid> <tx_tc> \
> >>>     tx <on|off> <rx_qid> <rx_tc> <pause_time>
> >>>
> >>
> >> Isn't the order of the paramters odd, it is mixing Rx/Tx config, 
> >> what about ordering Rx and Tx paramters?
> >>
> > It's been kept like this to portray config for rx_pause and tx_pause separately i.e. mode and corresponding config.
> >
>
> What do you mean 'separately'? You need to provide all arguments anyway, right?
>
> I was thinking first have the Rx arguments, later Tx, like:
>
> rx <on|off> <rx_qid> <rx_tc> tx <on|off> <tx_qid> <tx_tc> <pause_time>
I think this grouping is better.

>
> Am I missing something, is there a benefit of what you did in this patch?

Mentioned syntax takes input as per below config structure:
struct rte_eth_pfc_queue_conf {
        enum rte_eth_fc_mode mode; /**< Link flow control mode */

        struct {
                uint16_t tx_qid; /**< Tx queue ID */
                uint8_t tc; /**< Traffic class as per PFC (802.1Qbb) spec */
        } rx_pause; /* Valid when (mode == FC_RX_PAUSE || mode == FC_FULL) */

        struct {
                uint16_t pause_time; /**< Pause quota in the Pause frame */
                uint16_t rx_qid;     /**< Rx queue ID */
                uint8_t tc; /**< Traffic class as per PFC (802.1Qbb) spec */
        } tx_pause; /* Valid when (mode == FC_TX_PAUSE || mode == FC_FULL) */
};

First part represent first structure information and later part represent second structure information.
For the next version, I am keeping this syntax as it is and will be fixing some ethdev changes.

>
> >>> - Example command to configure queue based priority flow control
> >>>     on rx and tx side for port 0, Rx queue 0, Tx queue 0 with pause
> >>>     time 2047
> >>>
> >>> testpmd> set pfc_queue_ctrl 0 rx on 0 0 tx on 0 0 2047
> >>>
> >>> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> >>
> >> <...>
>