[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <PH0PR18MB4474BE62B79C293DFA866967DEC89@PH0PR18MB4474.namprd18.prod.outlook.com>
Date: Mon, 23 Jan 2023 17:26:27 +0000
From: Hariprasad Kelam <hkelam@...vell.com>
To: Simon Horman <simon.horman@...igine.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"kuba@...nel.org" <kuba@...nel.org>,
"davem@...emloft.net" <davem@...emloft.net>,
"pabeni@...hat.com" <pabeni@...hat.com>,
"edumazet@...gle.com" <edumazet@...gle.com>,
Sunil Kovvuri Goutham <sgoutham@...vell.com>,
Linu Cherian <lcherian@...vell.com>,
Geethasowjanya Akula <gakula@...vell.com>,
Jerin Jacob Kollanukkaran <jerinj@...vell.com>,
Subbaraya Sundeep Bhatta <sbhatta@...vell.com>,
"jhs@...atatu.com" <jhs@...atatu.com>,
"xiyou.wangcong@...il.com" <xiyou.wangcong@...il.com>,
"jiri@...nulli.us" <jiri@...nulli.us>,
"saeedm@...dia.com" <saeedm@...dia.com>,
"richardcochran@...il.com" <richardcochran@...il.com>,
"tariqt@...dia.com" <tariqt@...dia.com>,
"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
"maxtram95@...il.com" <maxtram95@...il.com>,
Naveen Mamindlapalli <naveenm@...vell.com>
Subject: Re: [net-next Patch v2 2/5] octeontx2-pf: qos send queues management
Thanks for the inputs, will address the review comments in the next version.
Thanks,
Hariprasad k
On Wed, Jan 18, 2023 at 04:21:04PM +0530, Hariprasad Kelam wrote:
> From: Subbaraya Sundeep <sbhatta@...vell.com>
>
> Current implementation is such that the number of Send queues (SQs)
> are decided on the device probe which is equal to the number of online
> cpus. These SQs are allocated and deallocated in interface open and c
> lose calls respectively.
>
> This patch defines new APIs for initializing and deinitializing Send
> queues dynamically and allocates more number of transmit queues for
> QOS feature.
>
> Signed-off-by: Subbaraya Sundeep <sbhatta@...vell.com>
> Signed-off-by: Hariprasad Kelam <hkelam@...vell.com>
> Signed-off-by: Sunil Kovvuri Goutham <sgoutham@...vell.com>
...
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> index 88f8772a61cd..0868ae825736 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> @@ -758,11 +758,16 @@ int otx2_txschq_stop(struct otx2_nic *pfvf)
> void otx2_sqb_flush(struct otx2_nic *pfvf) {
> int qidx, sqe_tail, sqe_head;
> + struct otx2_snd_queue *sq;
> u64 incr, *ptr, val;
> int timeout = 1000;
>
> ptr = (u64 *)otx2_get_regaddr(pfvf, NIX_LF_SQ_OP_STATUS);
> - for (qidx = 0; qidx < pfvf->hw.tot_tx_queues; qidx++) {
> + for (qidx = 0; qidx < pfvf->hw.tot_tx_queues +
> +pfvf->hw.tc_tx_queues;
nit:
It seems awkward that essentially this is saying that the total tx queues is 'tot_tx_queues' + 'tc_tx_queues'.
As I read 'tot' as being short for 'total'.
Also, the pfvf->hw.tot_tx_queues + pfvf->hw.tc_tx_queues pattern is rather verbose and repeated often. Perhaps a helper would... help.
Will add these changes in next version.
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> index c1ea60bc2630..3acda6d289d3 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
...
> @@ -1688,11 +1693,13 @@ int otx2_open(struct net_device *netdev)
>
> netif_carrier_off(netdev);
>
> - pf->qset.cq_cnt = pf->hw.rx_queues + pf->hw.tot_tx_queues;
> /* RQ and SQs are mapped to different CQs,
> * so find out max CQ IRQs (i.e CINTs) needed.
> */
> pf->hw.cint_cnt = max(pf->hw.rx_queues, pf->hw.tx_queues);
> + pf->hw.cint_cnt = max_t(u8, pf->hw.cint_cnt, pf->hw.tc_tx_queues);
nit: maybe this is nicer? *completely untested!*
pf->hw.cint_cnt = max3(pf->hw.rx_queues, pf->hw.tx_queues),
pf->hw.tc_tx_queues);
Will add these changes in next version.
...
> @@ -735,7 +741,10 @@ static void otx2_sqe_add_hdr(struct otx2_nic *pfvf, struct otx2_snd_queue *sq,
> sqe_hdr->aura = sq->aura_id;
> /* Post a CQE Tx after pkt transmission */
> sqe_hdr->pnc = 1;
> - sqe_hdr->sq = qidx;
> + if (pfvf->hw.tx_queues == qidx)
> + sqe_hdr->sq = qidx + pfvf->hw.xdp_queues;
> + else
> + sqe_hdr->sq = qidx;
nit: maybe this is nicer? *completely untested!*
sqe_hdr = pfvf->hw.tx_queues != qidx ?
qidx + pfvf->hw.xdp_queues : qidx;
Will add these changes in next version.
...
Powered by blists - more mailing lists