2023-08-09 08:17:06

by Suman Ghosh

[permalink] [raw]
Subject: [net PATCH V2 0/4] Fix PFC related issues

This patchset fixes multiple PFC related issues related to Octeon.

Patch #1: octeontx2-pf: Update PFC configuration

Patch #2: octeontx2-pf: Fix PFC TX scheduler free

Patch #3: octeontx2-af: CN10KB: fix PFC configuration

Patch #4: octeonxt2-pf: Fix backpressure config for multiple PFC
priorities to work simultaneously

Hariprasad Kelam (1):
octeontx2-af: CN10KB: fix PFC configuration

Suman Ghosh (3):
octeontx2-pf: Update PFC configuration
octeontx2-pf: Fix PFC TX scheduler free
octeonxt2-pf: Fix backpressure config for multiple PFC priorities to
work simultaneously

---
v2 changes:
1. Fixed compilation error in patch #2
ERROR: modpost: "otx2_txschq_free_one"
[drivers/net/ethernet/marvell/octeontx2/nic/rvu_nicvf.ko] undefined!
2. Added new patch #4 to the patch set. This patch fixes another PFC
related issue.

.../net/ethernet/marvell/octeontx2/af/rpm.c | 17 ++++-----
.../marvell/octeontx2/nic/otx2_common.c | 1 +
.../marvell/octeontx2/nic/otx2_dcbnl.c | 35 ++++++++-----------
3 files changed, 24 insertions(+), 29 deletions(-)

--
2.25.1



2023-08-09 08:23:36

by Suman Ghosh

[permalink] [raw]
Subject: [net PATCH V2 4/4] octeonxt2-pf: Fix backpressure config for multiple PFC priorities to work simultaneously

MAC (CGX or RPM) asserts backpressure at TL3 or TL2 node of the egress
hierarchical scheduler tree depending on link level config done. If
there are multiple PFC priorities enabled at a time and for all such
flows to backoff, each priority will have to assert backpressure at
different TL3/TL2 scheduler nodes and these flows will need to submit
egress pkts to these nodes.

Current PFC configuration has an issue where in only one backpressure
scheduler node is being allocated which is resulting in only one PFC
priority to work. This patch fixes this issue.

Fixes: 99c969a83d82 ("octeontx2-pf: Add egress PFC support")
Signed-off-by: Suman Ghosh <[email protected]>
---
.../net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c
index c75435bab411..048ee015c085 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c
@@ -70,7 +70,7 @@ static int otx2_pfc_txschq_alloc_one(struct otx2_nic *pfvf, u8 prio)
* link config level. These rest of the scheduler can be
* same as hw.txschq_list.
*/
- for (lvl = 0; lvl < pfvf->hw.txschq_link_cfg_lvl; lvl++)
+ for (lvl = 0; lvl <= pfvf->hw.txschq_link_cfg_lvl; lvl++)
req->schq[lvl] = 1;

rc = otx2_sync_mbox_msg(&pfvf->mbox);
@@ -83,7 +83,7 @@ static int otx2_pfc_txschq_alloc_one(struct otx2_nic *pfvf, u8 prio)
return PTR_ERR(rsp);

/* Setup transmit scheduler list */
- for (lvl = 0; lvl < pfvf->hw.txschq_link_cfg_lvl; lvl++) {
+ for (lvl = 0; lvl <= pfvf->hw.txschq_link_cfg_lvl; lvl++) {
if (!rsp->schq[lvl])
return -ENOSPC;

@@ -128,7 +128,7 @@ static int otx2_pfc_txschq_stop_one(struct otx2_nic *pfvf, u8 prio)
int lvl;

/* free PFC TLx nodes */
- for (lvl = 0; lvl < pfvf->hw.txschq_link_cfg_lvl; lvl++)
+ for (lvl = 0; lvl <= pfvf->hw.txschq_link_cfg_lvl; lvl++)
otx2_txschq_free_one(pfvf, lvl,
pfvf->pfc_schq_list[lvl][prio]);

@@ -400,9 +400,11 @@ static int otx2_dcbnl_ieee_setpfc(struct net_device *dev, struct ieee_pfc *pfc)
{
struct otx2_nic *pfvf = netdev_priv(dev);
bool if_up = netif_running(dev);
+ u8 prev_pfc_en;
int err;

/* Save PFC configuration to interface */
+ prev_pfc_en = pfvf->pfc_en;
pfvf->pfc_en = pfc->pfc_en;

if (pfvf->hw.tx_queues >= NIX_PF_PFC_PRIO_MAX)
@@ -421,7 +423,9 @@ static int otx2_dcbnl_ieee_setpfc(struct net_device *dev, struct ieee_pfc *pfc)
return err;

if (if_up) {
+ pfvf->pfc_en = prev_pfc_en;
otx2_stop(dev);
+ pfvf->pfc_en = pfc->pfc_en;
otx2_open(dev);
}

--
2.25.1


2023-08-09 09:01:50

by Suman Ghosh

[permalink] [raw]
Subject: [net PATCH V2 1/4] octeontx2-pf: Update PFC configuration

As of now we are creating/deleting Tx schedulers when user is
setting PFC on/off. The problem is if we have a running traffic on
the interface and as we are updating the sq->smq mapping on the fly,
we might loose completion interrupt for some packets. As a result of
that a watchdog reset is hit from BQL.
This patch solves the issue by simply calling interface off/on APIs
which will reconfigure all the queues. We might loss the running traffic
momentarily but that should be fine.

Fixes: 99c969a83d82 ("octeontx2-pf: Add egress PFC support")
Signed-off-by: Suman Ghosh <[email protected]>
---
.../net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c
index ccaf97bb1ce0..d54edfa8fcc9 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c
@@ -406,6 +406,7 @@ static int otx2_dcbnl_ieee_getpfc(struct net_device *dev, struct ieee_pfc *pfc)
static int otx2_dcbnl_ieee_setpfc(struct net_device *dev, struct ieee_pfc *pfc)
{
struct otx2_nic *pfvf = netdev_priv(dev);
+ bool if_up = netif_running(dev);
int err;

/* Save PFC configuration to interface */
@@ -426,14 +427,9 @@ static int otx2_dcbnl_ieee_setpfc(struct net_device *dev, struct ieee_pfc *pfc)
if (err)
return err;

- /* Request Per channel Bpids */
- if (pfc->pfc_en)
- otx2_nix_config_bp(pfvf, true);
-
- err = otx2_pfc_txschq_update(pfvf);
- if (err) {
- dev_err(pfvf->dev, "%s failed to update TX schedulers\n", __func__);
- return err;
+ if (if_up) {
+ otx2_stop(dev);
+ otx2_open(dev);
}

return 0;
--
2.25.1


2023-08-09 23:50:55

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [net PATCH V2 1/4] octeontx2-pf: Update PFC configuration

On Wed, 9 Aug 2023 12:35:29 +0530 Suman Ghosh wrote:
> + otx2_stop(dev);
> + otx2_open(dev);

If there is any error in open() this will silently take the interface
down. Can't you force a NAPI poll or some such, if the concern is a
missed IRQ?
--
pw-bot: cr

2023-08-19 08:26:26

by Suman Ghosh

[permalink] [raw]
Subject: RE: [EXT] Re: [net PATCH V2 1/4] octeontx2-pf: Update PFC configuration

>----------------------------------------------------------------------
>On Wed, 9 Aug 2023 12:35:29 +0530 Suman Ghosh wrote:
>> + otx2_stop(dev);
>> + otx2_open(dev);
>
>If there is any error in open() this will silently take the interface
>down. Can't you force a NAPI poll or some such, if the concern is a
>missed IRQ?
[Suman] I can check the return type of open() and report in case of error. But even if we force NAPI poll we might not be able to control the watchdog reset. If we have a running traffic and interface is up, when we force NAPI poll, then the new packets will have updated scheduler queue and we will still loose the completion interrupts of the previous packets. Do you see any issue if I handle the error situation during open() call?
>--
>pw-bot: cr