[net-next,v2,3/8] octeontx2-af: Disable backpressure between CPT and NIX

Commit Message

Bharat Bhushan May 13, 2024, 10:54 a.m. UTC

NIX can assert backpressure to CPT on the NIX<=>CPT link.
Keep the backpressure disabled for now. NIX block anyways
handles backpressure asserted by MAC due to PFC or flow
control pkts.

Signed-off-by: Bharat Bhushan <[e-mail]>
---
 .../net/ethernet/marvell/octeontx2/af/mbox.h  |  4 +
 .../ethernet/marvell/octeontx2/af/rvu_nix.c   | 74 ++++++++++++++++---
 .../marvell/octeontx2/nic/otx2_common.c       | 25 +++++++
 .../marvell/octeontx2/nic/otx2_common.h       |  1 +
 .../marvell/octeontx2/nic/otx2_dcbnl.c        |  3 +
 .../ethernet/marvell/octeontx2/nic/otx2_pf.c  |  3 +
 6 files changed, 100 insertions(+), 10 deletions(-)

Comments

Simon Horman May 13, 2024, 4:14 p.m. UTC | #1

On Mon, May 13, 2024 at 04:24:41PM +0530, Bharat Bhushan wrote:
> NIX can assert backpressure to CPT on the NIX<=>CPT link.
> Keep the backpressure disabled for now. NIX block anyways
> handles backpressure asserted by MAC due to PFC or flow
> control pkts.
> 
> Signed-off-by: Bharat Bhushan <[e-mail]>

...

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c

...

> @@ -592,8 +596,16 @@ int rvu_mbox_handler_nix_bp_disable(struct rvu *rvu,
>  	bp = &nix_hw->bp;
>  	chan_base = pfvf->rx_chan_base + req->chan_base;
>  	for (chan = chan_base; chan < (chan_base + req->chan_cnt); chan++) {
> -		cfg = rvu_read64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan));
> -		rvu_write64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan),
> +		/* CPT channel for a given link channel is always
> +		 * assumed to be BIT(11) set in link channel.
> +		 */
> +		if (cpt_link)
> +			chan_v = chan | BIT(11);
> +		else
> +			chan_v = chan;

Hi Bharat,

The chan_v logic above seems to appear twice in this patch.
I'd suggest adding a helper.

> +
> +		cfg = rvu_read64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan_v));
> +		rvu_write64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan_v),
>  			    cfg & ~BIT_ULL(16));
>  
>  		if (type == NIX_INTF_TYPE_LBK) {

...

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> index 7ec99c8d610c..e9d2e039a322 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> @@ -1705,6 +1705,31 @@ int otx2_nix_config_bp(struct otx2_nic *pfvf, bool enable)
>  }
>  EXPORT_SYMBOL(otx2_nix_config_bp);
>  
> +int otx2_nix_cpt_config_bp(struct otx2_nic *pfvf, bool enable)
> +{
> +	struct nix_bp_cfg_req *req;
> +
> +	if (enable)
> +		req = otx2_mbox_alloc_msg_nix_cpt_bp_enable(&pfvf->mbox);
> +	else
> +		req = otx2_mbox_alloc_msg_nix_cpt_bp_disable(&pfvf->mbox);
> +
> +	if (!req)
> +		return -ENOMEM;
> +
> +	req->chan_base = 0;
> +#ifdef CONFIG_DCB
> +	req->chan_cnt = pfvf->pfc_en ? IEEE_8021QAZ_MAX_TCS : 1;
> +	req->bpid_per_chan = pfvf->pfc_en ? 1 : 0;
> +#else
> +	req->chan_cnt =  1;
> +	req->bpid_per_chan = 0;
> +#endif

IMHO, inline #ifdefs reduce readability and reduce maintainability.

Would it be possible to either:

1. Include the pfc_en field in struct otx2_nic and make
   sure it is set to 0 if CONFIG_DCB is unset; or
2. Provide a wrapper that returns 0 if CONFIG_DCB is unset,
   otherwise pfvf->pfc_en.

I suspect 1 will have little downside and be easiest to implement.

> +
> +	return otx2_sync_mbox_msg(&pfvf->mbox);
> +}
> +EXPORT_SYMBOL(otx2_nix_cpt_config_bp);
> +
>  /* Mbox message handlers */
>  void mbox_handler_cgx_stats(struct otx2_nic *pfvf,
>  			    struct cgx_stats_rsp *rsp)

...

Bharat Bhushan May 14, 2024, 6:39 a.m. UTC | #2

Please see inline

> -----Original Message-----
> From: Simon Horman <[e-mail]>
> Sent: Monday, May 13, 2024 9:45 PM
> To: Bharat Bhushan <[e-mail]>
> Cc: [e-mail]; [e-mail]; Sunil Kovvuri
> Goutham <[e-mail]>; Geethasowjanya Akula
> <[e-mail]>; Subbaraya Sundeep Bhatta <[e-mail]>;
> Hariprasad Kelam <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]; [e-mail]; Jerin Jacob
> <[e-mail]>; Linu Cherian <[e-mail]>;
> [e-mail]
> Subject: [EXTERNAL] Re: [net-next,v2 3/8] octeontx2-af: Disable backpressure
> between CPT and NIX
> 
> 
> ----------------------------------------------------------------------
> On Mon, May 13, 2024 at 04:24:41PM +0530, Bharat Bhushan wrote:
> > NIX can assert backpressure to CPT on the NIX<=>CPT link.
> > Keep the backpressure disabled for now. NIX block anyways
> > handles backpressure asserted by MAC due to PFC or flow
> > control pkts.
> >
> > Signed-off-by: Bharat Bhushan <[e-mail]>
> 
> ...
> 
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> 
> ...
> 
> > @@ -592,8 +596,16 @@ int rvu_mbox_handler_nix_bp_disable(struct rvu
> *rvu,
> >  	bp = &nix_hw->bp;
> >  	chan_base = pfvf->rx_chan_base + req->chan_base;
> >  	for (chan = chan_base; chan < (chan_base + req->chan_cnt); chan++) {
> > -		cfg = rvu_read64(rvu, blkaddr,
> NIX_AF_RX_CHANX_CFG(chan));
> > -		rvu_write64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan),
> > +		/* CPT channel for a given link channel is always
> > +		 * assumed to be BIT(11) set in link channel.
> > +		 */
> > +		if (cpt_link)
> > +			chan_v = chan | BIT(11);
> > +		else
> > +			chan_v = chan;
> 
> Hi Bharat,
> 
> The chan_v logic above seems to appear twice in this patch.
> I'd suggest adding a helper.

Will fix in next version.

> 
> > +
> > +		cfg = rvu_read64(rvu, blkaddr,
> NIX_AF_RX_CHANX_CFG(chan_v));
> > +		rvu_write64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan_v),
> >  			    cfg & ~BIT_ULL(16));
> >
> >  		if (type == NIX_INTF_TYPE_LBK) {
> 
> ...
> 
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > index 7ec99c8d610c..e9d2e039a322 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > @@ -1705,6 +1705,31 @@ int otx2_nix_config_bp(struct otx2_nic *pfvf,
> bool enable)
> >  }
> >  EXPORT_SYMBOL(otx2_nix_config_bp);
> >
> > +int otx2_nix_cpt_config_bp(struct otx2_nic *pfvf, bool enable)
> > +{
> > +	struct nix_bp_cfg_req *req;
> > +
> > +	if (enable)
> > +		req = otx2_mbox_alloc_msg_nix_cpt_bp_enable(&pfvf-
> >mbox);
> > +	else
> > +		req = otx2_mbox_alloc_msg_nix_cpt_bp_disable(&pfvf-
> >mbox);
> > +
> > +	if (!req)
> > +		return -ENOMEM;
> > +
> > +	req->chan_base = 0;
> > +#ifdef CONFIG_DCB
> > +	req->chan_cnt = pfvf->pfc_en ? IEEE_8021QAZ_MAX_TCS : 1;
> > +	req->bpid_per_chan = pfvf->pfc_en ? 1 : 0;
> > +#else
> > +	req->chan_cnt =  1;
> > +	req->bpid_per_chan = 0;
> > +#endif
> 
> IMHO, inline #ifdefs reduce readability and reduce maintainability.
> 
> Would it be possible to either:
> 
> 1. Include the pfc_en field in struct otx2_nic and make
>    sure it is set to 0 if CONFIG_DCB is unset; or
> 2. Provide a wrapper that returns 0 if CONFIG_DCB is unset,
>    otherwise pfvf->pfc_en.
> 
> I suspect 1 will have little downside and be easiest to implement.

pfc_en is already a field of otx2_nic but under CONFIG_DCB. Will fix by adding a wrapper function like:

static bool is_pfc_enabled(struct otx2_nic *pfvf)
{
#ifdef CONFIG_DCB
        return pfvf->pfc_en ? true : false;
#endif
        return false;
}

Using same like..
...
        if (is_pfc_enabled(pfvf)) {
                req->chan_cnt = IEEE_8021QAZ_MAX_TCS;
                req->bpid_per_chan = 1;
        } else {
                req->chan_cnt = 1;
                req->bpid_per_chan = 0;
        }
...

Thanks
-Bharat

> 
> > +
> > +	return otx2_sync_mbox_msg(&pfvf->mbox);
> > +}
> > +EXPORT_SYMBOL(otx2_nix_cpt_config_bp);
> > +
> >  /* Mbox message handlers */
> >  void mbox_handler_cgx_stats(struct otx2_nic *pfvf,
> >  			    struct cgx_stats_rsp *rsp)
> 
> ...

Simon Horman May 14, 2024, 10:41 a.m. UTC | #3

On Tue, May 14, 2024 at 06:39:45AM +0000, Bharat Bhushan wrote:
> Please see inline
> 
> > -----Original Message-----
> > From: Simon Horman <[e-mail]>
> > Sent: Monday, May 13, 2024 9:45 PM
> > To: Bharat Bhushan <[e-mail]>
> > Cc: [e-mail]; [e-mail]; Sunil Kovvuri
> > Goutham <[e-mail]>; Geethasowjanya Akula
> > <[e-mail]>; Subbaraya Sundeep Bhatta <[e-mail]>;
> > Hariprasad Kelam <[e-mail]>; [e-mail];
> > [e-mail]; [e-mail]; [e-mail]; Jerin Jacob
> > <[e-mail]>; Linu Cherian <[e-mail]>;
> > [e-mail]
> > Subject: [EXTERNAL] Re: [net-next,v2 3/8] octeontx2-af: Disable backpressure
> > between CPT and NIX
> > 
> > 
> > ----------------------------------------------------------------------
> > On Mon, May 13, 2024 at 04:24:41PM +0530, Bharat Bhushan wrote:
> > > NIX can assert backpressure to CPT on the NIX<=>CPT link.
> > > Keep the backpressure disabled for now. NIX block anyways
> > > handles backpressure asserted by MAC due to PFC or flow
> > > control pkts.
> > >
> > > Signed-off-by: Bharat Bhushan <[e-mail]>
> > 
> > ...
> > 
> > > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> > b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> > 
> > ...
> > 
> > > @@ -592,8 +596,16 @@ int rvu_mbox_handler_nix_bp_disable(struct rvu
> > *rvu,
> > >  	bp = &nix_hw->bp;
> > >  	chan_base = pfvf->rx_chan_base + req->chan_base;
> > >  	for (chan = chan_base; chan < (chan_base + req->chan_cnt); chan++) {
> > > -		cfg = rvu_read64(rvu, blkaddr,
> > NIX_AF_RX_CHANX_CFG(chan));
> > > -		rvu_write64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan),
> > > +		/* CPT channel for a given link channel is always
> > > +		 * assumed to be BIT(11) set in link channel.
> > > +		 */
> > > +		if (cpt_link)
> > > +			chan_v = chan | BIT(11);
> > > +		else
> > > +			chan_v = chan;
> > 
> > Hi Bharat,
> > 
> > The chan_v logic above seems to appear twice in this patch.
> > I'd suggest adding a helper.
> 
> Will fix in next version.
> 
> > 
> > > +
> > > +		cfg = rvu_read64(rvu, blkaddr,
> > NIX_AF_RX_CHANX_CFG(chan_v));
> > > +		rvu_write64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan_v),
> > >  			    cfg & ~BIT_ULL(16));
> > >
> > >  		if (type == NIX_INTF_TYPE_LBK) {
> > 
> > ...
> > 
> > > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > > index 7ec99c8d610c..e9d2e039a322 100644
> > > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > > @@ -1705,6 +1705,31 @@ int otx2_nix_config_bp(struct otx2_nic *pfvf,
> > bool enable)
> > >  }
> > >  EXPORT_SYMBOL(otx2_nix_config_bp);
> > >
> > > +int otx2_nix_cpt_config_bp(struct otx2_nic *pfvf, bool enable)
> > > +{
> > > +	struct nix_bp_cfg_req *req;
> > > +
> > > +	if (enable)
> > > +		req = otx2_mbox_alloc_msg_nix_cpt_bp_enable(&pfvf-
> > >mbox);
> > > +	else
> > > +		req = otx2_mbox_alloc_msg_nix_cpt_bp_disable(&pfvf-
> > >mbox);
> > > +
> > > +	if (!req)
> > > +		return -ENOMEM;
> > > +
> > > +	req->chan_base = 0;
> > > +#ifdef CONFIG_DCB
> > > +	req->chan_cnt = pfvf->pfc_en ? IEEE_8021QAZ_MAX_TCS : 1;
> > > +	req->bpid_per_chan = pfvf->pfc_en ? 1 : 0;
> > > +#else
> > > +	req->chan_cnt =  1;
> > > +	req->bpid_per_chan = 0;
> > > +#endif
> > 
> > IMHO, inline #ifdefs reduce readability and reduce maintainability.
> > 
> > Would it be possible to either:
> > 
> > 1. Include the pfc_en field in struct otx2_nic and make
> >    sure it is set to 0 if CONFIG_DCB is unset; or
> > 2. Provide a wrapper that returns 0 if CONFIG_DCB is unset,
> >    otherwise pfvf->pfc_en.
> > 
> > I suspect 1 will have little downside and be easiest to implement.
> 
> pfc_en is already a field of otx2_nic but under CONFIG_DCB. Will fix by adding a wrapper function like:

Thanks. Just to clarify, my first suggestion was to move
pfc_en outside of CONFIG_DCB in otx2_nic.

> 
> static bool is_pfc_enabled(struct otx2_nic *pfvf)
> {
> #ifdef CONFIG_DCB
>         return pfvf->pfc_en ? true : false;

FWIIW, I think this could also be:

	return !!pfvf->pfc_en;

> #endif
>         return false;
> }

Also, I do wonder if the following can work:

	return IS_ENABLED(CONFIG_DCB) && pfvf->pfc_en;

> 
> Using same like..
> ...
>         if (is_pfc_enabled(pfvf)) {

If so, perhaps this can work:

	if (IS_ENABLED(CONFIG_DCB) && pfvf->pfc_en) {
		...

>                 req->chan_cnt = IEEE_8021QAZ_MAX_TCS;
>                 req->bpid_per_chan = 1;
>         } else {
>                 req->chan_cnt = 1;
>                 req->bpid_per_chan = 0;
>         }
> ...

Bharat Bhushan May 14, 2024, 11:26 a.m. UTC | #4

Please see inline

> -----Original Message-----
> From: Simon Horman <[e-mail]>
> Sent: Tuesday, May 14, 2024 4:11 PM
> To: Bharat Bhushan <[e-mail]>
> Cc: [e-mail]; [e-mail]; Sunil Kovvuri
> Goutham <[e-mail]>; Geethasowjanya Akula
> <[e-mail]>; Subbaraya Sundeep Bhatta <[e-mail]>;
> Hariprasad Kelam <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]; [e-mail]; Jerin Jacob
> <[e-mail]>; Linu Cherian <[e-mail]>;
> [e-mail]
> Subject: Re: [EXTERNAL] Re: [net-next,v2 3/8] octeontx2-af: Disable
> backpressure between CPT and NIX
> 
> On Tue, May 14, 2024 at 06:39:45AM +0000, Bharat Bhushan wrote:
> > Please see inline
> >
> > > -----Original Message-----
> > > From: Simon Horman <[e-mail]>
> > > Sent: Monday, May 13, 2024 9:45 PM
> > > To: Bharat Bhushan <[e-mail]>
> > > Cc: [e-mail]; [e-mail]; Sunil
> > > Kovvuri Goutham <[e-mail]>; Geethasowjanya Akula
> > > <[e-mail]>; Subbaraya Sundeep Bhatta
> > > <[e-mail]>; Hariprasad Kelam <[e-mail]>;
> > > [e-mail]; [e-mail]; [e-mail];
> > > [e-mail]; Jerin Jacob <[e-mail]>; Linu Cherian
> > > <[e-mail]>; [e-mail]
> > > Subject: [EXTERNAL] Re: [net-next,v2 3/8] octeontx2-af: Disable
> > > backpressure between CPT and NIX
> > >
> > >
> > > --------------------------------------------------------------------
> > > -- On Mon, May 13, 2024 at 04:24:41PM +0530, Bharat Bhushan wrote:
> > > > NIX can assert backpressure to CPT on the NIX<=>CPT link.
> > > > Keep the backpressure disabled for now. NIX block anyways handles
> > > > backpressure asserted by MAC due to PFC or flow control pkts.
> > > >
> > > > Signed-off-by: Bharat Bhushan <[e-mail]>
> > >
> > > ...
> > >
> > > > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> > > b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> > >
> > > ...
> > >
> > > > @@ -592,8 +596,16 @@ int rvu_mbox_handler_nix_bp_disable(struct
> > > > rvu
> > > *rvu,
> > > >  	bp = &nix_hw->bp;
> > > >  	chan_base = pfvf->rx_chan_base + req->chan_base;
> > > >  	for (chan = chan_base; chan < (chan_base + req->chan_cnt); chan++) {
> > > > -		cfg = rvu_read64(rvu, blkaddr,
> > > NIX_AF_RX_CHANX_CFG(chan));
> > > > -		rvu_write64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan),
> > > > +		/* CPT channel for a given link channel is always
> > > > +		 * assumed to be BIT(11) set in link channel.
> > > > +		 */
> > > > +		if (cpt_link)
> > > > +			chan_v = chan | BIT(11);
> > > > +		else
> > > > +			chan_v = chan;
> > >
> > > Hi Bharat,
> > >
> > > The chan_v logic above seems to appear twice in this patch.
> > > I'd suggest adding a helper.
> >
> > Will fix in next version.
> >
> > >
> > > > +
> > > > +		cfg = rvu_read64(rvu, blkaddr,
> > > NIX_AF_RX_CHANX_CFG(chan_v));
> > > > +		rvu_write64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan_v),
> > > >  			    cfg & ~BIT_ULL(16));
> > > >
> > > >  		if (type == NIX_INTF_TYPE_LBK) {
> > >
> > > ...
> > >
> > > > diff --git
> > > > a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > > b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > > > index 7ec99c8d610c..e9d2e039a322 100644
> > > > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > > > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> > > > @@ -1705,6 +1705,31 @@ int otx2_nix_config_bp(struct otx2_nic
> > > > *pfvf,
> > > bool enable)
> > > >  }
> > > >  EXPORT_SYMBOL(otx2_nix_config_bp);
> > > >
> > > > +int otx2_nix_cpt_config_bp(struct otx2_nic *pfvf, bool enable) {
> > > > +	struct nix_bp_cfg_req *req;
> > > > +
> > > > +	if (enable)
> > > > +		req = otx2_mbox_alloc_msg_nix_cpt_bp_enable(&pfvf-
> > > >mbox);
> > > > +	else
> > > > +		req = otx2_mbox_alloc_msg_nix_cpt_bp_disable(&pfvf-
> > > >mbox);
> > > > +
> > > > +	if (!req)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	req->chan_base = 0;
> > > > +#ifdef CONFIG_DCB
> > > > +	req->chan_cnt = pfvf->pfc_en ? IEEE_8021QAZ_MAX_TCS : 1;
> > > > +	req->bpid_per_chan = pfvf->pfc_en ? 1 : 0; #else
> > > > +	req->chan_cnt =  1;
> > > > +	req->bpid_per_chan = 0;
> > > > +#endif
> > >
> > > IMHO, inline #ifdefs reduce readability and reduce maintainability.
> > >
> > > Would it be possible to either:
> > >
> > > 1. Include the pfc_en field in struct otx2_nic and make
> > >    sure it is set to 0 if CONFIG_DCB is unset; or 2. Provide a
> > > wrapper that returns 0 if CONFIG_DCB is unset,
> > >    otherwise pfvf->pfc_en.
> > >
> > > I suspect 1 will have little downside and be easiest to implement.
> >
> > pfc_en is already a field of otx2_nic but under CONFIG_DCB. Will fix by
> adding a wrapper function like:
> 
> Thanks. Just to clarify, my first suggestion was to move pfc_en outside of
> CONFIG_DCB in otx2_nic.
> 
> >
> > static bool is_pfc_enabled(struct otx2_nic *pfvf) { #ifdef CONFIG_DCB
> >         return pfvf->pfc_en ? true : false;
> 
> FWIIW, I think this could also be:
> 
> 	return !!pfvf->pfc_en;
> 
> > #endif
> >         return false;
> > }
> 
> Also, I do wonder if the following can work:
> 
> 	return IS_ENABLED(CONFIG_DCB) && pfvf->pfc_en;

This is required at more than one place, so will keep wrapper function with this condition check.

Thanks
-Bharat

> 
> >
> > Using same like..
> > ...
> >         if (is_pfc_enabled(pfvf)) {
> 
> If so, perhaps this can work:
> 
> 	if (IS_ENABLED(CONFIG_DCB) && pfvf->pfc_en) {
> 		...
> 
> >                 req->chan_cnt = IEEE_8021QAZ_MAX_TCS;
> >                 req->bpid_per_chan = 1;
> >         } else {
> >                 req->chan_cnt = 1;
> >                 req->bpid_per_chan = 0;
> >         }
> > ...

Simon Horman May 14, 2024, 11:45 a.m. UTC | #5

On Tue, May 14, 2024 at 11:26:54AM +0000, Bharat Bhushan wrote:
> Please see inline
> 
> > -----Original Message-----
> > From: Simon Horman <[e-mail]>

...

> > > > I suspect 1 will have little downside and be easiest to implement.
> > >
> > > pfc_en is already a field of otx2_nic but under CONFIG_DCB. Will fix by
> > adding a wrapper function like:
> > 
> > Thanks. Just to clarify, my first suggestion was to move pfc_en outside of
> > CONFIG_DCB in otx2_nic.
> > 
> > >
> > > static bool is_pfc_enabled(struct otx2_nic *pfvf) { #ifdef CONFIG_DCB
> > >         return pfvf->pfc_en ? true : false;
> > 
> > FWIIW, I think this could also be:
> > 
> > 	return !!pfvf->pfc_en;
> > 
> > > #endif
> > >         return false;
> > > }
> > 
> > Also, I do wonder if the following can work:
> > 
> > 	return IS_ENABLED(CONFIG_DCB) && pfvf->pfc_en;
> 
> This is required at more than one place, so will keep wrapper function with this condition check.

Thanks, sounds good.

...

Patch

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
index 4a77f6fe2622..0cb399b8d2ca 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
@@ -309,6 +309,10 @@  M(NIX_BANDPROF_FREE,	0x801e, nix_bandprof_free, nix_bandprof_free_req,   \
 				msg_rsp)				    \
 M(NIX_BANDPROF_GET_HWINFO, 0x801f, nix_bandprof_get_hwinfo, msg_req,		\
 				nix_bandprof_get_hwinfo_rsp)		    \
+M(NIX_CPT_BP_ENABLE,    0x8020, nix_cpt_bp_enable, nix_bp_cfg_req,	    \
+				nix_bp_cfg_rsp)				    \
+M(NIX_CPT_BP_DISABLE,   0x8021, nix_cpt_bp_disable, nix_bp_cfg_req,	    \
+				msg_rsp)				\
 M(NIX_READ_INLINE_IPSEC_CFG, 0x8023, nix_read_inline_ipsec_cfg,		\
 				msg_req, nix_inline_ipsec_cfg)		\
 M(NIX_MCAST_GRP_CREATE,	0x802b, nix_mcast_grp_create, nix_mcast_grp_create_req,	\
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
index 00af8888e329..fbb45993ad0c 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
@@ -567,9 +567,9 @@  void rvu_nix_flr_free_bpids(struct rvu *rvu, u16 pcifunc)
 	mutex_unlock(&rvu->rsrc_lock);
 }
 
-int rvu_mbox_handler_nix_bp_disable(struct rvu *rvu,
-				    struct nix_bp_cfg_req *req,
-				    struct msg_rsp *rsp)
+static int nix_bp_disable(struct rvu *rvu,
+			  struct nix_bp_cfg_req *req,
+			  struct msg_rsp *rsp, bool cpt_link)
 {
 	u16 pcifunc = req->hdr.pcifunc;
 	int blkaddr, pf, type, err;
@@ -577,6 +577,7 @@  int rvu_mbox_handler_nix_bp_disable(struct rvu *rvu,
 	struct rvu_pfvf *pfvf;
 	struct nix_hw *nix_hw;
 	struct nix_bp *bp;
+	u16 chan_v;
 	u64 cfg;
 
 	pf = rvu_get_pf(pcifunc);
@@ -584,6 +585,9 @@  int rvu_mbox_handler_nix_bp_disable(struct rvu *rvu,
 	if (!is_pf_cgxmapped(rvu, pf) && type != NIX_INTF_TYPE_LBK)
 		return 0;
 
+	if (cpt_link && !rvu->hw->cpt_links)
+		return 0;
+
 	pfvf = rvu_get_pfvf(rvu, pcifunc);
 	err = nix_get_struct_ptrs(rvu, pcifunc, &nix_hw, &blkaddr);
 	if (err)
@@ -592,8 +596,16 @@  int rvu_mbox_handler_nix_bp_disable(struct rvu *rvu,
 	bp = &nix_hw->bp;
 	chan_base = pfvf->rx_chan_base + req->chan_base;
 	for (chan = chan_base; chan < (chan_base + req->chan_cnt); chan++) {
-		cfg = rvu_read64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan));
-		rvu_write64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan),
+		/* CPT channel for a given link channel is always
+		 * assumed to be BIT(11) set in link channel.
+		 */
+		if (cpt_link)
+			chan_v = chan | BIT(11);
+		else
+			chan_v = chan;
+
+		cfg = rvu_read64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan_v));
+		rvu_write64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan_v),
 			    cfg & ~BIT_ULL(16));
 
 		if (type == NIX_INTF_TYPE_LBK) {
@@ -612,6 +624,20 @@  int rvu_mbox_handler_nix_bp_disable(struct rvu *rvu,
 	return 0;
 }
 
+int rvu_mbox_handler_nix_bp_disable(struct rvu *rvu,
+				    struct nix_bp_cfg_req *req,
+				    struct msg_rsp *rsp)
+{
+	return nix_bp_disable(rvu, req, rsp, false);
+}
+
+int rvu_mbox_handler_nix_cpt_bp_disable(struct rvu *rvu,
+					struct nix_bp_cfg_req *req,
+					struct msg_rsp *rsp)
+{
+	return nix_bp_disable(rvu, req, rsp, true);
+}
+
 static int rvu_nix_get_bpid(struct rvu *rvu, struct nix_bp_cfg_req *req,
 			    int type, int chan_id)
 {
@@ -691,15 +717,17 @@  static int rvu_nix_get_bpid(struct rvu *rvu, struct nix_bp_cfg_req *req,
 	return bpid;
 }
 
-int rvu_mbox_handler_nix_bp_enable(struct rvu *rvu,
-				   struct nix_bp_cfg_req *req,
-				   struct nix_bp_cfg_rsp *rsp)
+static int nix_bp_enable(struct rvu *rvu,
+			 struct nix_bp_cfg_req *req,
+			 struct nix_bp_cfg_rsp *rsp,
+			 bool cpt_link)
 {
 	int blkaddr, pf, type, chan_id = 0;
 	u16 pcifunc = req->hdr.pcifunc;
 	struct rvu_pfvf *pfvf;
 	u16 chan_base, chan;
 	s16 bpid, bpid_base;
+	u16 chan_v;
 	u64 cfg;
 
 	pf = rvu_get_pf(pcifunc);
@@ -712,6 +740,9 @@  int rvu_mbox_handler_nix_bp_enable(struct rvu *rvu,
 	    type != NIX_INTF_TYPE_SDP)
 		return 0;
 
+	if (cpt_link && !rvu->hw->cpt_links)
+		return 0;
+
 	pfvf = rvu_get_pfvf(rvu, pcifunc);
 	blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, pcifunc);
 
@@ -725,9 +756,18 @@  int rvu_mbox_handler_nix_bp_enable(struct rvu *rvu,
 			return -EINVAL;
 		}
 
-		cfg = rvu_read64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan));
+		/* CPT channel for a given link channel is always
+		 * assumed to be BIT(11) set in link channel.
+		 */
+
+		if (cpt_link)
+			chan_v = chan | BIT(11);
+		else
+			chan_v = chan;
+
+		cfg = rvu_read64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan_v));
 		cfg &= ~GENMASK_ULL(8, 0);
-		rvu_write64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan),
+		rvu_write64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(chan_v),
 			    cfg | (bpid & GENMASK_ULL(8, 0)) | BIT_ULL(16));
 		chan_id++;
 		bpid = rvu_nix_get_bpid(rvu, req, type, chan_id);
@@ -745,6 +785,20 @@  int rvu_mbox_handler_nix_bp_enable(struct rvu *rvu,
 	return 0;
 }
 
+int rvu_mbox_handler_nix_bp_enable(struct rvu *rvu,
+				   struct nix_bp_cfg_req *req,
+				   struct nix_bp_cfg_rsp *rsp)
+{
+	return nix_bp_enable(rvu, req, rsp, false);
+}
+
+int rvu_mbox_handler_nix_cpt_bp_enable(struct rvu *rvu,
+				       struct nix_bp_cfg_req *req,
+				       struct nix_bp_cfg_rsp *rsp)
+{
+	return nix_bp_enable(rvu, req, rsp, true);
+}
+
 static void nix_setup_lso_tso_l3(struct rvu *rvu, int blkaddr,
 				 u64 format, bool v4, u64 *fidx)
 {
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
index 7ec99c8d610c..e9d2e039a322 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
@@ -1705,6 +1705,31 @@  int otx2_nix_config_bp(struct otx2_nic *pfvf, bool enable)
 }
 EXPORT_SYMBOL(otx2_nix_config_bp);
 
+int otx2_nix_cpt_config_bp(struct otx2_nic *pfvf, bool enable)
+{
+	struct nix_bp_cfg_req *req;
+
+	if (enable)
+		req = otx2_mbox_alloc_msg_nix_cpt_bp_enable(&pfvf->mbox);
+	else
+		req = otx2_mbox_alloc_msg_nix_cpt_bp_disable(&pfvf->mbox);
+
+	if (!req)
+		return -ENOMEM;
+
+	req->chan_base = 0;
+#ifdef CONFIG_DCB
+	req->chan_cnt = pfvf->pfc_en ? IEEE_8021QAZ_MAX_TCS : 1;
+	req->bpid_per_chan = pfvf->pfc_en ? 1 : 0;
+#else
+	req->chan_cnt =  1;
+	req->bpid_per_chan = 0;
+#endif
+
+	return otx2_sync_mbox_msg(&pfvf->mbox);
+}
+EXPORT_SYMBOL(otx2_nix_cpt_config_bp);
+
 /* Mbox message handlers */
 void mbox_handler_cgx_stats(struct otx2_nic *pfvf,
 			    struct cgx_stats_rsp *rsp)
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
index 99b480e21e1c..42a759a33c11 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
@@ -987,6 +987,7 @@  int otx2_alloc_rbuf(struct otx2_nic *pfvf, struct otx2_pool *pool,
 int otx2_rxtx_enable(struct otx2_nic *pfvf, bool enable);
 void otx2_ctx_disable(struct mbox *mbox, int type, bool npa);
 int otx2_nix_config_bp(struct otx2_nic *pfvf, bool enable);
+int otx2_nix_cpt_config_bp(struct otx2_nic *pfvf, bool enable);
 void otx2_cleanup_rx_cqes(struct otx2_nic *pfvf, struct otx2_cq_queue *cq, int qidx);
 void otx2_cleanup_tx_cqes(struct otx2_nic *pfvf, struct otx2_cq_queue *cq);
 int otx2_sq_init(struct otx2_nic *pfvf, u16 qidx, u16 sqb_aura);
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c
index 28fb643d2917..da28725adcf8 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c
@@ -424,6 +424,9 @@  static int otx2_dcbnl_ieee_setpfc(struct net_device *dev, struct ieee_pfc *pfc)
 		return err;
 	}
 
+	/* Default disable backpressure on NIX-CPT */
+	otx2_nix_cpt_config_bp(pfvf, false);
+
 	/* Request Per channel Bpids */
 	if (pfc->pfc_en)
 		otx2_nix_config_bp(pfvf, true);
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
index f5bce3e326cc..cbd5050f58e8 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
@@ -1509,6 +1509,9 @@  static int otx2_init_hw_resources(struct otx2_nic *pf)
 	if (err)
 		goto err_free_npa_lf;
 
+	/* Default disable backpressure on NIX-CPT */
+	otx2_nix_cpt_config_bp(pf, false);
+
 	/* Enable backpressure for CGX mapped PF/VFs */
 	if (!is_otx2_lbkvf(pf->pdev))
 		otx2_nix_config_bp(pf, true);