* Re: [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode support @ 2021-02-04 15:33 Hariprasad Kelam 0 siblings, 0 replies; 4+ messages in thread From: Hariprasad Kelam @ 2021-02-04 15:33 UTC (permalink / raw) To: Jakub Kicinski Cc: netdev, linux-kernel, davem, willemdebruijn.kernel, andrew, Sunil Kovvuri Goutham, Linu Cherian, Geethasowjanya Akula, Jerin Jacob Kollanukkaran, Subbaraya Sundeep Bhatta Hi Jakub, > -----Original Message----- > From: Jakub Kicinski <kuba@kernel.org> > Sent: Wednesday, February 3, 2021 6:42 AM > To: Hariprasad Kelam <hkelam@marvell.com> > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > davem@davemloft.net; willemdebruijn.kernel@gmail.com; > andrew@lunn.ch; Sunil Kovvuri Goutham <sgoutham@marvell.com>; Linu > Cherian <lcherian@marvell.com>; Geethasowjanya Akula > <gakula@marvell.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; > Subbaraya Sundeep Bhatta <sbhatta@marvell.com> > Subject: [EXT] Re: [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode > support > > On Mon, 1 Feb 2021 10:54:40 +0530 Hariprasad Kelam wrote: > > From: Christina Jacob <cjacob@marvell.com> > > > > Add ethtool support to configure fec modes baser/rs and support to > > fecth FEC stats from CGX as well PHY. > > > > Configure fec mode > > - ethtool --set-fec eth0 encoding rs/baser/off/auto Query fec mode > > - ethtool --show-fec eth0 > > > + if (pfvf->linfo.fec) { > > + sprintf(data, "Fec Corrected Errors: "); > > + data += ETH_GSTRING_LEN; > > + sprintf(data, "Fec Uncorrected Errors: "); > > + data += ETH_GSTRING_LEN; > > Once again, you can't dynamically hide stats. ethtool makes 3 separate > system calls - to get the number of stats, get the names, and get the values. If > someone changes the FEC config in between those user space dumping stats > will get confused. > Agreed. Will fix this in next version. > > + } > > } > > > +static int otx2_get_fecparam(struct net_device *netdev, > > + struct ethtool_fecparam *fecparam) { > > + struct otx2_nic *pfvf = netdev_priv(netdev); > > + struct cgx_fw_data *rsp; > > + const int fec[] = { > > + ETHTOOL_FEC_OFF, > > + ETHTOOL_FEC_BASER, > > + ETHTOOL_FEC_RS, > > + ETHTOOL_FEC_BASER | ETHTOOL_FEC_RS}; #define > FEC_MAX_INDEX 3 > > + if (pfvf->linfo.fec < FEC_MAX_INDEX) > > This should be < Agreed . Current code miss the "ETHTOOL_FEC_BASER | ETHTOOL_FEC_RS" condition, will fix this in next version. > > > + fecparam->active_fec = fec[pfvf->linfo.fec]; > > > > + rsp = otx2_get_fwdata(pfvf); > > + if (IS_ERR(rsp)) > > + return PTR_ERR(rsp); > > + > > + if (rsp->fwdata.supported_fec <= FEC_MAX_INDEX) { > > + if (!rsp->fwdata.supported_fec) > > + fecparam->fec = ETHTOOL_FEC_NONE; > > + else > > + fecparam->fec = fec[rsp->fwdata.supported_fec]; > > + } > > + return 0; > > +} > > + > > +static int otx2_set_fecparam(struct net_device *netdev, > > + struct ethtool_fecparam *fecparam) { > > + struct otx2_nic *pfvf = netdev_priv(netdev); > > + struct mbox *mbox = &pfvf->mbox; > > + struct fec_mode *req, *rsp; > > + int err = 0, fec = 0; > > + > > + switch (fecparam->fec) { > > + /* Firmware does not support AUTO mode consider it as FEC_NONE > */ > > + case ETHTOOL_FEC_OFF: > > + case ETHTOOL_FEC_AUTO: > > + case ETHTOOL_FEC_NONE: > > I _think_ NONE is for drivers to report that they don't support FEC settings. > It's an output only parameter. On input OFF should be used. > Thanks for pointing this. Cross checked code also _NONE is output is only parameter. Will fix in next version. > > + fec = OTX2_FEC_NONE; > > + break; > > + case ETHTOOL_FEC_RS: > > + fec = OTX2_FEC_RS; > > + break; > > + case ETHTOOL_FEC_BASER: > > + fec = OTX2_FEC_BASER; > > + break; > > + default: > > + netdev_warn(pfvf->netdev, "Unsupported FEC mode: %d", > > + fecparam->fec); > > + return -EINVAL; > > + } > > + > > + if (fec == pfvf->linfo.fec) > > + return 0; > > + > > + mutex_lock(&mbox->lock); > > + req = otx2_mbox_alloc_msg_cgx_set_fec_param(&pfvf->mbox); > > + if (!req) { > > + err = -ENOMEM; > > + goto end; > > + } > > + req->fec = fec; > > + err = otx2_sync_mbox_msg(&pfvf->mbox); > > + if (err) > > + goto end; > > + > > + rsp = (struct fec_mode *)otx2_mbox_get_rsp(&pfvf->mbox.mbox, > > + 0, &req->hdr); > > + if (rsp->fec >= 0) { > > + pfvf->linfo.fec = rsp->fec; > > + /* clear stale counters */ > > + pfvf->hw.cgx_fec_corr_blks = 0; > > + pfvf->hw.cgx_fec_uncorr_blks = 0; > > Stats are supposed to be cumulative. Don't reset the stats just because > someone changed the FEC mode. You can miss errors this way. > Thanks for pointing this. Will fix in next version. Thanks, Hariprasad k > > + } else { > > + err = rsp->fec; > > + } ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Patch v3 net-next 0/7] ethtool support for fec and link configuration
@ 2021-02-01 5:24 Hariprasad Kelam
2021-02-01 5:24 ` [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode support Hariprasad Kelam
0 siblings, 1 reply; 4+ messages in thread
From: Hariprasad Kelam @ 2021-02-01 5:24 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: kuba, davem, willemdebruijn.kernel, andrew, sgoutham, lcherian,
gakula, jerinj, sbhatta, hkelam
This series of patches add support for forward error correction(fec) and
physical link configuration. Patches 1&2 adds necessary mbox handlers for fec
mode configuration request and to fetch stats. Patch 3 registers driver
callbacks for fec mode configuration and display. Patch 4&5 adds support of mbox
handlers for configuring link parameters like speed/duplex and autoneg etc.
Patche 6&7 registers driver callbacks for physical link configuration.
Change-log:
v2:
- Fixed review comments
- Corrected indentation issues
- Return -ENOMEM incase of mbox allocation failure
- added validation for input fecparams bitmask values
- added more comments
V3:
- Removed inline functions
- Make use of ethtool helpers APIs to display supported
advertised modes
- corrected indentation issues
- code changes such that return early in case of failure
to aid branch prediction
Christina Jacob (6):
octeontx2-af: forward error correction configuration
octeontx2-pf: ethtool fec mode support
octeontx2-af: Physical link configuration support
octeontx2-af: advertised link modes support on cgx
octeontx2-pf: ethtool physical link status
octeontx2-pf: ethtool physical link configuration
Felix Manlunas (1):
octeontx2-af: Add new CGX_CMD to get PHY FEC statistics
drivers/net/ethernet/marvell/octeontx2/af/cgx.c | 258 ++++++++++++-
drivers/net/ethernet/marvell/octeontx2/af/cgx.h | 10 +
.../net/ethernet/marvell/octeontx2/af/cgx_fw_if.h | 70 +++-
drivers/net/ethernet/marvell/octeontx2/af/mbox.h | 87 ++++-
drivers/net/ethernet/marvell/octeontx2/af/rvu.h | 4 +
.../net/ethernet/marvell/octeontx2/af/rvu_cgx.c | 80 +++++
.../ethernet/marvell/octeontx2/nic/otx2_common.c | 20 ++
.../ethernet/marvell/octeontx2/nic/otx2_common.h | 6 +
.../ethernet/marvell/octeontx2/nic/otx2_ethtool.c | 399 ++++++++++++++++++++-
.../net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 3 +
10 files changed, 930 insertions(+), 7 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 4+ messages in thread* [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode support 2021-02-01 5:24 [Patch v3 net-next 0/7] ethtool support for fec and link configuration Hariprasad Kelam @ 2021-02-01 5:24 ` Hariprasad Kelam 2021-02-03 0:44 ` Jesse Brandeburg 2021-02-03 1:12 ` Jakub Kicinski 0 siblings, 2 replies; 4+ messages in thread From: Hariprasad Kelam @ 2021-02-01 5:24 UTC (permalink / raw) To: netdev, linux-kernel Cc: kuba, davem, willemdebruijn.kernel, andrew, sgoutham, lcherian, gakula, jerinj, sbhatta, hkelam From: Christina Jacob <cjacob@marvell.com> Add ethtool support to configure fec modes baser/rs and support to fecth FEC stats from CGX as well PHY. Configure fec mode - ethtool --set-fec eth0 encoding rs/baser/off/auto Query fec mode - ethtool --show-fec eth0 Signed-off-by: Christina Jacob <cjacob@marvell.com> Signed-off-by: Sunil Goutham <sgoutham@marvell.com> Signed-off-by: Hariprasad Kelam <hkelam@marvell.com> --- .../ethernet/marvell/octeontx2/nic/otx2_common.c | 20 +++ .../ethernet/marvell/octeontx2/nic/otx2_common.h | 6 + .../ethernet/marvell/octeontx2/nic/otx2_ethtool.c | 181 ++++++++++++++++++++- .../net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 3 + 4 files changed, 208 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c index 5ddedc3..1e67072 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c @@ -60,6 +60,19 @@ void otx2_update_lmac_stats(struct otx2_nic *pfvf) mutex_unlock(&pfvf->mbox.lock); } +void otx2_update_lmac_fec_stats(struct otx2_nic *pfvf) +{ + struct msg_req *req; + + if (!netif_running(pfvf->netdev)) + return; + mutex_lock(&pfvf->mbox.lock); + req = otx2_mbox_alloc_msg_cgx_fec_stats(&pfvf->mbox); + if (req) + otx2_sync_mbox_msg(&pfvf->mbox); + mutex_unlock(&pfvf->mbox.lock); +} + int otx2_update_rq_stats(struct otx2_nic *pfvf, int qidx) { struct otx2_rcv_queue *rq = &pfvf->qset.rq[qidx]; @@ -1492,6 +1505,13 @@ void mbox_handler_cgx_stats(struct otx2_nic *pfvf, pfvf->hw.cgx_tx_stats[id] = rsp->tx_stats[id]; } +void mbox_handler_cgx_fec_stats(struct otx2_nic *pfvf, + struct cgx_fec_stats_rsp *rsp) +{ + pfvf->hw.cgx_fec_corr_blks += rsp->fec_corr_blks; + pfvf->hw.cgx_fec_uncorr_blks += rsp->fec_uncorr_blks; +} + void mbox_handler_nix_txsch_alloc(struct otx2_nic *pf, struct nix_txsch_alloc_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 143ae04..b3f3de9 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h @@ -204,6 +204,8 @@ struct otx2_hw { struct otx2_drv_stats drv_stats; u64 cgx_rx_stats[CGX_RX_STATS_COUNT]; u64 cgx_tx_stats[CGX_TX_STATS_COUNT]; + u64 cgx_fec_corr_blks; + u64 cgx_fec_uncorr_blks; u8 cgx_links; /* No. of CGX links present in HW */ u8 lbk_links; /* No. of LBK links present in HW */ }; @@ -660,6 +662,9 @@ void mbox_handler_nix_txsch_alloc(struct otx2_nic *pf, struct nix_txsch_alloc_rsp *rsp); void mbox_handler_cgx_stats(struct otx2_nic *pfvf, struct cgx_stats_rsp *rsp); +void mbox_handler_cgx_fec_stats(struct otx2_nic *pfvf, + struct cgx_fec_stats_rsp *rsp); +void otx2_set_fec_stats_count(struct otx2_nic *pfvf); void mbox_handler_nix_bp_enable(struct otx2_nic *pfvf, struct nix_bp_cfg_rsp *rsp); @@ -668,6 +673,7 @@ void otx2_get_dev_stats(struct otx2_nic *pfvf); void otx2_get_stats64(struct net_device *netdev, struct rtnl_link_stats64 *stats); void otx2_update_lmac_stats(struct otx2_nic *pfvf); +void otx2_update_lmac_fec_stats(struct otx2_nic *pfvf); int otx2_update_rq_stats(struct otx2_nic *pfvf, int qidx); int otx2_update_sq_stats(struct otx2_nic *pfvf, int qidx); void otx2_set_ethtool_ops(struct net_device *netdev); diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c index e0199f0..e5b1a57 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c @@ -66,6 +66,8 @@ static const unsigned int otx2_n_dev_stats = ARRAY_SIZE(otx2_dev_stats); static const unsigned int otx2_n_drv_stats = ARRAY_SIZE(otx2_drv_stats); static const unsigned int otx2_n_queue_stats = ARRAY_SIZE(otx2_queue_stats); +static struct cgx_fw_data *otx2_get_fwdata(struct otx2_nic *pfvf); + static void otx2_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *info) { @@ -128,6 +130,12 @@ static void otx2_get_strings(struct net_device *netdev, u32 sset, u8 *data) strcpy(data, "reset_count"); data += ETH_GSTRING_LEN; + if (pfvf->linfo.fec) { + sprintf(data, "Fec Corrected Errors: "); + data += ETH_GSTRING_LEN; + sprintf(data, "Fec Uncorrected Errors: "); + data += ETH_GSTRING_LEN; + } } static void otx2_get_qset_stats(struct otx2_nic *pfvf, @@ -160,11 +168,30 @@ static void otx2_get_qset_stats(struct otx2_nic *pfvf, } } +static int otx2_get_phy_fec_stats(struct otx2_nic *pfvf) +{ + struct msg_req *req; + int rc = -ENOMEM; + + mutex_lock(&pfvf->mbox.lock); + req = otx2_mbox_alloc_msg_cgx_get_phy_fec_stats(&pfvf->mbox); + if (!req) + goto end; + + if (!otx2_sync_mbox_msg(&pfvf->mbox)) + rc = 0; +end: + mutex_unlock(&pfvf->mbox.lock); + return rc; +} + /* Get device and per queue statistics */ static void otx2_get_ethtool_stats(struct net_device *netdev, struct ethtool_stats *stats, u64 *data) { struct otx2_nic *pfvf = netdev_priv(netdev); + u64 fec_corr_blks, fec_uncorr_blks; + struct cgx_fw_data *rsp; int stat; otx2_get_dev_stats(pfvf); @@ -183,12 +210,43 @@ static void otx2_get_ethtool_stats(struct net_device *netdev, for (stat = 0; stat < CGX_TX_STATS_COUNT; stat++) *(data++) = pfvf->hw.cgx_tx_stats[stat]; *(data++) = pfvf->reset_count; + + /* Do not request fec stats if interface fec mode is none */ + if (pfvf->linfo.fec == OTX2_FEC_NONE) + return; + + fec_corr_blks = pfvf->hw.cgx_fec_corr_blks; + fec_uncorr_blks = pfvf->hw.cgx_fec_uncorr_blks; + + rsp = otx2_get_fwdata(pfvf); + if (!IS_ERR(rsp) && rsp->fwdata.phy.misc.has_fec_stats && + !otx2_get_phy_fec_stats(pfvf)) { + /* Fetch fwdata again because it's been recently populated with + * latest PHY FEC stats. + */ + rsp = otx2_get_fwdata(pfvf); + if (!IS_ERR(rsp)) { + struct fec_stats_s *p = &rsp->fwdata.phy.fec_stats; + + if (pfvf->linfo.fec == OTX2_FEC_BASER) { + fec_corr_blks = p->brfec_corr_blks; + fec_uncorr_blks = p->brfec_uncorr_blks; + } else { + fec_corr_blks = p->rsfec_corr_cws; + fec_uncorr_blks = p->rsfec_uncorr_cws; + } + } + } + + *(data++) = fec_corr_blks; + *(data++) = fec_uncorr_blks; } static int otx2_get_sset_count(struct net_device *netdev, int sset) { struct otx2_nic *pfvf = netdev_priv(netdev); - int qstats_count; + int qstats_count, fec_stats_count = 0; + bool if_up = netif_running(netdev); if (sset != ETH_SS_STATS) return -EINVAL; @@ -196,8 +254,16 @@ static int otx2_get_sset_count(struct net_device *netdev, int sset) qstats_count = otx2_n_queue_stats * (pfvf->hw.rx_queues + pfvf->hw.tx_queues); + /* Do not show fec stats if interface fec mode is none */ + if (!if_up || !pfvf->linfo.fec) + return otx2_n_dev_stats + otx2_n_drv_stats + qstats_count + + CGX_RX_STATS_COUNT + CGX_TX_STATS_COUNT + 1; + + fec_stats_count = 2; + otx2_update_lmac_fec_stats(pfvf); + return otx2_n_dev_stats + otx2_n_drv_stats + qstats_count + - CGX_RX_STATS_COUNT + CGX_TX_STATS_COUNT + 1; + CGX_RX_STATS_COUNT + CGX_TX_STATS_COUNT + 1 + fec_stats_count; } /* Get no of queues device supports and current queue count */ @@ -859,6 +925,115 @@ static int otx2_get_ts_info(struct net_device *netdev, return 0; } +static struct cgx_fw_data *otx2_get_fwdata(struct otx2_nic *pfvf) +{ + struct cgx_fw_data *rsp = NULL; + struct msg_req *req; + int err = 0; + + mutex_lock(&pfvf->mbox.lock); + req = otx2_mbox_alloc_msg_cgx_get_aux_link_info(&pfvf->mbox); + if (!req) { + mutex_unlock(&pfvf->mbox.lock); + return ERR_PTR(-ENOMEM); + } + + err = otx2_sync_mbox_msg(&pfvf->mbox); + if (!err) { + rsp = (struct cgx_fw_data *) + otx2_mbox_get_rsp(&pfvf->mbox.mbox, 0, &req->hdr); + } else { + rsp = ERR_PTR(err); + } + + mutex_unlock(&pfvf->mbox.lock); + return rsp; +} + +static int otx2_get_fecparam(struct net_device *netdev, + struct ethtool_fecparam *fecparam) +{ + struct otx2_nic *pfvf = netdev_priv(netdev); + struct cgx_fw_data *rsp; + const int fec[] = { + ETHTOOL_FEC_OFF, + ETHTOOL_FEC_BASER, + ETHTOOL_FEC_RS, + ETHTOOL_FEC_BASER | ETHTOOL_FEC_RS}; +#define FEC_MAX_INDEX 3 + if (pfvf->linfo.fec < FEC_MAX_INDEX) + fecparam->active_fec = fec[pfvf->linfo.fec]; + + rsp = otx2_get_fwdata(pfvf); + if (IS_ERR(rsp)) + return PTR_ERR(rsp); + + if (rsp->fwdata.supported_fec <= FEC_MAX_INDEX) { + if (!rsp->fwdata.supported_fec) + fecparam->fec = ETHTOOL_FEC_NONE; + else + fecparam->fec = fec[rsp->fwdata.supported_fec]; + } + return 0; +} + +static int otx2_set_fecparam(struct net_device *netdev, + struct ethtool_fecparam *fecparam) +{ + struct otx2_nic *pfvf = netdev_priv(netdev); + struct mbox *mbox = &pfvf->mbox; + struct fec_mode *req, *rsp; + int err = 0, fec = 0; + + switch (fecparam->fec) { + /* Firmware does not support AUTO mode consider it as FEC_NONE */ + case ETHTOOL_FEC_OFF: + case ETHTOOL_FEC_AUTO: + case ETHTOOL_FEC_NONE: + fec = OTX2_FEC_NONE; + break; + case ETHTOOL_FEC_RS: + fec = OTX2_FEC_RS; + break; + case ETHTOOL_FEC_BASER: + fec = OTX2_FEC_BASER; + break; + default: + netdev_warn(pfvf->netdev, "Unsupported FEC mode: %d", + fecparam->fec); + return -EINVAL; + } + + if (fec == pfvf->linfo.fec) + return 0; + + mutex_lock(&mbox->lock); + req = otx2_mbox_alloc_msg_cgx_set_fec_param(&pfvf->mbox); + if (!req) { + err = -ENOMEM; + goto end; + } + req->fec = fec; + err = otx2_sync_mbox_msg(&pfvf->mbox); + if (err) + goto end; + + rsp = (struct fec_mode *)otx2_mbox_get_rsp(&pfvf->mbox.mbox, + 0, &req->hdr); + if (rsp->fec >= 0) { + pfvf->linfo.fec = rsp->fec; + /* clear stale counters */ + pfvf->hw.cgx_fec_corr_blks = 0; + pfvf->hw.cgx_fec_uncorr_blks = 0; + } else { + err = rsp->fec; + } + +end: + mutex_unlock(&mbox->lock); + return err; +} + static const struct ethtool_ops otx2_ethtool_ops = { .supported_coalesce_params = ETHTOOL_COALESCE_USECS | ETHTOOL_COALESCE_MAX_FRAMES, @@ -886,6 +1061,8 @@ static const struct ethtool_ops otx2_ethtool_ops = { .get_pauseparam = otx2_get_pauseparam, .set_pauseparam = otx2_set_pauseparam, .get_ts_info = otx2_get_ts_info, + .get_fecparam = otx2_get_fecparam, + .set_fecparam = otx2_set_fecparam, }; void otx2_set_ethtool_ops(struct net_device *netdev) diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c index 07ec85a..d024dac 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c @@ -779,6 +779,9 @@ static void otx2_process_pfaf_mbox_msg(struct otx2_nic *pf, case MBOX_MSG_CGX_STATS: mbox_handler_cgx_stats(pf, (struct cgx_stats_rsp *)msg); break; + case MBOX_MSG_CGX_FEC_STATS: + mbox_handler_cgx_fec_stats(pf, (struct cgx_fec_stats_rsp *)msg); + break; default: if (msg->rc) dev_err(pf->dev, -- 2.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode support 2021-02-01 5:24 ` [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode support Hariprasad Kelam @ 2021-02-03 0:44 ` Jesse Brandeburg 2021-02-03 1:12 ` Jakub Kicinski 1 sibling, 0 replies; 4+ messages in thread From: Jesse Brandeburg @ 2021-02-03 0:44 UTC (permalink / raw) To: Hariprasad Kelam Cc: netdev, linux-kernel, kuba, davem, willemdebruijn.kernel, andrew, sgoutham, lcherian, gakula, jerinj, sbhatta Hariprasad Kelam wrote: > From: Christina Jacob <cjacob@marvell.com> > > Add ethtool support to configure fec modes baser/rs and > support to fecth FEC stats from CGX as well PHY. > > Configure fec mode > - ethtool --set-fec eth0 encoding rs/baser/off/auto > Query fec mode > - ethtool --show-fec eth0 > > Signed-off-by: Christina Jacob <cjacob@marvell.com> > Signed-off-by: Sunil Goutham <sgoutham@marvell.com> > Signed-off-by: Hariprasad Kelam <hkelam@marvell.com> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode support 2021-02-01 5:24 ` [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode support Hariprasad Kelam 2021-02-03 0:44 ` Jesse Brandeburg @ 2021-02-03 1:12 ` Jakub Kicinski 1 sibling, 0 replies; 4+ messages in thread From: Jakub Kicinski @ 2021-02-03 1:12 UTC (permalink / raw) To: Hariprasad Kelam Cc: netdev, linux-kernel, davem, willemdebruijn.kernel, andrew, sgoutham, lcherian, gakula, jerinj, sbhatta On Mon, 1 Feb 2021 10:54:40 +0530 Hariprasad Kelam wrote: > From: Christina Jacob <cjacob@marvell.com> > > Add ethtool support to configure fec modes baser/rs and > support to fecth FEC stats from CGX as well PHY. > > Configure fec mode > - ethtool --set-fec eth0 encoding rs/baser/off/auto > Query fec mode > - ethtool --show-fec eth0 > + if (pfvf->linfo.fec) { > + sprintf(data, "Fec Corrected Errors: "); > + data += ETH_GSTRING_LEN; > + sprintf(data, "Fec Uncorrected Errors: "); > + data += ETH_GSTRING_LEN; Once again, you can't dynamically hide stats. ethtool makes 3 separate system calls - to get the number of stats, get the names, and get the values. If someone changes the FEC config in between those user space dumping stats will get confused. > + } > } > +static int otx2_get_fecparam(struct net_device *netdev, > + struct ethtool_fecparam *fecparam) > +{ > + struct otx2_nic *pfvf = netdev_priv(netdev); > + struct cgx_fw_data *rsp; > + const int fec[] = { > + ETHTOOL_FEC_OFF, > + ETHTOOL_FEC_BASER, > + ETHTOOL_FEC_RS, > + ETHTOOL_FEC_BASER | ETHTOOL_FEC_RS}; > +#define FEC_MAX_INDEX 3 > + if (pfvf->linfo.fec < FEC_MAX_INDEX) This should be < > + fecparam->active_fec = fec[pfvf->linfo.fec]; > + rsp = otx2_get_fwdata(pfvf); > + if (IS_ERR(rsp)) > + return PTR_ERR(rsp); > + > + if (rsp->fwdata.supported_fec <= FEC_MAX_INDEX) { > + if (!rsp->fwdata.supported_fec) > + fecparam->fec = ETHTOOL_FEC_NONE; > + else > + fecparam->fec = fec[rsp->fwdata.supported_fec]; > + } > + return 0; > +} > + > +static int otx2_set_fecparam(struct net_device *netdev, > + struct ethtool_fecparam *fecparam) > +{ > + struct otx2_nic *pfvf = netdev_priv(netdev); > + struct mbox *mbox = &pfvf->mbox; > + struct fec_mode *req, *rsp; > + int err = 0, fec = 0; > + > + switch (fecparam->fec) { > + /* Firmware does not support AUTO mode consider it as FEC_NONE */ > + case ETHTOOL_FEC_OFF: > + case ETHTOOL_FEC_AUTO: > + case ETHTOOL_FEC_NONE: I _think_ NONE is for drivers to report that they don't support FEC settings. It's an output only parameter. On input OFF should be used. > + fec = OTX2_FEC_NONE; > + break; > + case ETHTOOL_FEC_RS: > + fec = OTX2_FEC_RS; > + break; > + case ETHTOOL_FEC_BASER: > + fec = OTX2_FEC_BASER; > + break; > + default: > + netdev_warn(pfvf->netdev, "Unsupported FEC mode: %d", > + fecparam->fec); > + return -EINVAL; > + } > + > + if (fec == pfvf->linfo.fec) > + return 0; > + > + mutex_lock(&mbox->lock); > + req = otx2_mbox_alloc_msg_cgx_set_fec_param(&pfvf->mbox); > + if (!req) { > + err = -ENOMEM; > + goto end; > + } > + req->fec = fec; > + err = otx2_sync_mbox_msg(&pfvf->mbox); > + if (err) > + goto end; > + > + rsp = (struct fec_mode *)otx2_mbox_get_rsp(&pfvf->mbox.mbox, > + 0, &req->hdr); > + if (rsp->fec >= 0) { > + pfvf->linfo.fec = rsp->fec; > + /* clear stale counters */ > + pfvf->hw.cgx_fec_corr_blks = 0; > + pfvf->hw.cgx_fec_uncorr_blks = 0; Stats are supposed to be cumulative. Don't reset the stats just because someone changed the FEC mode. You can miss errors this way. > + } else { > + err = rsp->fec; > + } ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-02-04 15:37 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-04 15:33 [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode support Hariprasad Kelam -- strict thread matches above, loose matches on Subject: below -- 2021-02-01 5:24 [Patch v3 net-next 0/7] ethtool support for fec and link configuration Hariprasad Kelam 2021-02-01 5:24 ` [Patch v3 net-next 3/7] octeontx2-pf: ethtool fec mode support Hariprasad Kelam 2021-02-03 0:44 ` Jesse Brandeburg 2021-02-03 1:12 ` Jakub Kicinski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.