* [PATCH net-next 0/3] nfp: support FEC mode reporting and auto-neg
@ 2022-09-21 12:12 Simon Horman
2022-09-21 12:12 ` [PATCH net-next 1/3] nfp: add support for reporting active FEC mode Simon Horman
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Simon Horman @ 2022-09-21 12:12 UTC (permalink / raw)
To: David Miller, Jakub Kicinski, Paolo Abeni; +Cc: netdev, oss-drivers, Fei Qin
Hi,
this series adds support for the following features to the nfp driver:
* Patch 1/3: Support active FEC mode
* Patch 2/3: Support link auto negotiation
* Patch 3/3: Support restart of link auto negotiation
Fei Qin (1):
nfp: add support restart of link auto-negotiation
Yinjun Zhang (2):
nfp: add support for reporting active FEC mode
nfp: add support for link auto negotiation
.../ethernet/netronome/nfp/nfp_net_ethtool.c | 78 +++++++++++++++++--
.../net/ethernet/netronome/nfp/nfp_net_main.c | 23 +++++-
.../ethernet/netronome/nfp/nfpcore/nfp_nsp.h | 4 +
.../netronome/nfp/nfpcore/nfp_nsp_eth.c | 13 +++-
4 files changed, 107 insertions(+), 11 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH net-next 1/3] nfp: add support for reporting active FEC mode 2022-09-21 12:12 [PATCH net-next 0/3] nfp: support FEC mode reporting and auto-neg Simon Horman @ 2022-09-21 12:12 ` Simon Horman 2022-09-21 12:12 ` [PATCH net-next 2/3] nfp: add support for link auto negotiation Simon Horman 2022-09-21 12:12 ` [PATCH net-next 3/3] nfp: add support restart of link auto-negotiation Simon Horman 2 siblings, 0 replies; 17+ messages in thread From: Simon Horman @ 2022-09-21 12:12 UTC (permalink / raw) To: David Miller, Jakub Kicinski, Paolo Abeni; +Cc: netdev, oss-drivers, Fei Qin From: Yinjun Zhang <yinjun.zhang@corigine.com> The latest management firmware can now report the active FEC mode. Adapt driver accordingly so that user can get the active FEC mode by running command: # ethtool --show-fec <intf> Also correct use of `fec` field. Signed-off-by: Yinjun Zhang <yinjun.zhang@corigine.com> Reviewed-by: Louis Peens <louis.peens@corigine.com> Signed-off-by: Simon Horman <simon.horman@corigine.com> --- drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c | 2 +- drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h | 2 ++ drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c | 9 ++++++++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c index db58532364b6..d50af23642a2 100644 --- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c @@ -996,7 +996,7 @@ nfp_port_get_fecparam(struct net_device *netdev, return 0; param->fec = nfp_port_fec_nsp_to_ethtool(eth_port->fec_modes_supported); - param->active_fec = nfp_port_fec_nsp_to_ethtool(eth_port->fec); + param->active_fec = nfp_port_fec_nsp_to_ethtool(BIT(eth_port->act_fec)); return 0; } diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h index 77d66855be42..52465670a01e 100644 --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h @@ -132,6 +132,7 @@ enum nfp_eth_fec { * @ports.interface: interface (module) plugged in * @ports.media: media type of the @interface * @ports.fec: forward error correction mode + * @ports.act_fec: active forward error correction mode * @ports.aneg: auto negotiation mode * @ports.mac_addr: interface MAC address * @ports.label_port: port id @@ -162,6 +163,7 @@ struct nfp_eth_table { enum nfp_eth_media media; enum nfp_eth_fec fec; + enum nfp_eth_fec act_fec; enum nfp_eth_aneg aneg; u8 mac_addr[ETH_ALEN]; diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c index 4cc38799eabc..18ba7629cdc2 100644 --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c @@ -40,6 +40,7 @@ #define NSP_ETH_STATE_OVRD_CHNG BIT_ULL(22) #define NSP_ETH_STATE_ANEG GENMASK_ULL(25, 23) #define NSP_ETH_STATE_FEC GENMASK_ULL(27, 26) +#define NSP_ETH_STATE_ACT_FEC GENMASK_ULL(29, 28) #define NSP_ETH_CTRL_CONFIGURED BIT_ULL(0) #define NSP_ETH_CTRL_ENABLED BIT_ULL(1) @@ -170,7 +171,13 @@ nfp_eth_port_translate(struct nfp_nsp *nsp, const union eth_table_entry *src, if (dst->fec_modes_supported) dst->fec_modes_supported |= NFP_FEC_AUTO | NFP_FEC_DISABLED; - dst->fec = 1 << FIELD_GET(NSP_ETH_STATE_FEC, state); + dst->fec = FIELD_GET(NSP_ETH_STATE_FEC, state); + dst->act_fec = dst->fec; + + if (nfp_nsp_get_abi_ver_minor(nsp) < 33) + return; + + dst->act_fec = FIELD_GET(NSP_ETH_STATE_ACT_FEC, state); } static void -- 2.30.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 2/3] nfp: add support for link auto negotiation 2022-09-21 12:12 [PATCH net-next 0/3] nfp: support FEC mode reporting and auto-neg Simon Horman 2022-09-21 12:12 ` [PATCH net-next 1/3] nfp: add support for reporting active FEC mode Simon Horman @ 2022-09-21 12:12 ` Simon Horman 2022-09-23 1:00 ` Jakub Kicinski 2022-09-21 12:12 ` [PATCH net-next 3/3] nfp: add support restart of link auto-negotiation Simon Horman 2 siblings, 1 reply; 17+ messages in thread From: Simon Horman @ 2022-09-21 12:12 UTC (permalink / raw) To: David Miller, Jakub Kicinski, Paolo Abeni; +Cc: netdev, oss-drivers, Fei Qin From: Yinjun Zhang <yinjun.zhang@corigine.com> Report the auto negotiation capability if it's supported in management firmware, and advertise it if it's enabled. Changing FEC to mode other than auto or changing port speed is not allowed when autoneg is enabled. And FEC mode is enforced into auto mode when enabling link autoneg. The ethtool <intf> command displays the auto-neg capability: # ethtool enp1s0np0 Settings for enp1s0np0: Supported ports: [ FIBRE ] Supported link modes: Not reported Supported pause frame use: Symmetric Supports auto-negotiation: Yes Supported FEC modes: None RS BASER Advertised link modes: Not reported Advertised pause frame use: Symmetric Advertised auto-negotiation: Yes Advertised FEC modes: None RS BASER Speed: 25000Mb/s Duplex: Full Auto-negotiation: on Port: FIBRE PHYAD: 0 Transceiver: internal Link detected: yes Signed-off-by: Yinjun Zhang <yinjun.zhang@corigine.com> Reviewed-by: Louis Peens <louis.peens@corigine.com> Signed-off-by: Simon Horman <simon.horman@corigine.com> --- .../ethernet/netronome/nfp/nfp_net_ethtool.c | 43 ++++++++++++++++--- .../net/ethernet/netronome/nfp/nfp_net_main.c | 23 +++++++++- .../ethernet/netronome/nfp/nfpcore/nfp_nsp.h | 2 + .../netronome/nfp/nfpcore/nfp_nsp_eth.c | 4 +- 4 files changed, 63 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c index d50af23642a2..00aacc48a7a2 100644 --- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c @@ -290,8 +290,13 @@ nfp_net_get_link_ksettings(struct net_device *netdev, if (eth_port) { ethtool_link_ksettings_add_link_mode(cmd, supported, Pause); ethtool_link_ksettings_add_link_mode(cmd, advertising, Pause); - cmd->base.autoneg = eth_port->aneg != NFP_ANEG_DISABLED ? - AUTONEG_ENABLE : AUTONEG_DISABLE; + if (eth_port->supp_aneg) { + ethtool_link_ksettings_add_link_mode(cmd, supported, Autoneg); + if (eth_port->aneg == NFP_ANEG_AUTO) { + ethtool_link_ksettings_add_link_mode(cmd, advertising, Autoneg); + cmd->base.autoneg = AUTONEG_ENABLE; + } + } nfp_net_set_fec_link_mode(eth_port, cmd); } @@ -327,6 +332,7 @@ static int nfp_net_set_link_ksettings(struct net_device *netdev, const struct ethtool_link_ksettings *cmd) { + bool req_aneg = (cmd->base.autoneg == AUTONEG_ENABLE); struct nfp_eth_table_port *eth_port; struct nfp_port *port; struct nfp_nsp *nsp; @@ -346,16 +352,36 @@ nfp_net_set_link_ksettings(struct net_device *netdev, if (IS_ERR(nsp)) return PTR_ERR(nsp); - err = __nfp_eth_set_aneg(nsp, cmd->base.autoneg == AUTONEG_ENABLE ? - NFP_ANEG_AUTO : NFP_ANEG_DISABLED); + if (req_aneg && !eth_port->supp_aneg) { + netdev_warn(netdev, "Autoneg is not supported.\n"); + err = -EOPNOTSUPP; + goto err_bad_set; + } + + err = __nfp_eth_set_aneg(nsp, req_aneg ? NFP_ANEG_AUTO : NFP_ANEG_DISABLED); if (err) goto err_bad_set; + if (cmd->base.speed != SPEED_UNKNOWN) { - u32 speed = cmd->base.speed / eth_port->lanes; + if (req_aneg) { + netdev_err(netdev, "Speed changing is not allowed when working on autoneg mode.\n"); + err = -EINVAL; + goto err_bad_set; + } else { + u32 speed = cmd->base.speed / eth_port->lanes; - err = __nfp_eth_set_speed(nsp, speed); + err = __nfp_eth_set_speed(nsp, speed); + if (err) + goto err_bad_set; + } + } + + if (req_aneg && nfp_eth_can_support_fec(eth_port) && eth_port->fec != NFP_FEC_AUTO_BIT) { + err = __nfp_eth_set_fec(nsp, NFP_FEC_AUTO_BIT); if (err) goto err_bad_set; + + netdev_info(netdev, "FEC is enforced into auto mode when autoneg is enabled.\n"); } err = nfp_eth_config_commit_end(nsp); @@ -1021,6 +1047,11 @@ nfp_port_set_fecparam(struct net_device *netdev, if (fec < 0) return fec; + if (eth_port->supp_aneg && eth_port->aneg == NFP_ANEG_AUTO && fec != NFP_FEC_AUTO_BIT) { + netdev_err(netdev, "Only auto mode is allowed when link autoneg is enabled.\n"); + return -EINVAL; + } + err = nfp_eth_set_fec(port->app->cpp, eth_port->index, fec); if (!err) /* Only refresh if we did something */ diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c index e2d4c487e8de..2c0279dcf299 100644 --- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c @@ -322,8 +322,11 @@ static int nfp_net_pf_cfg_nsp(struct nfp_pf *pf, bool sp_indiff) snprintf(hwinfo, sizeof(hwinfo), "sp_indiff=%d", sp_indiff); err = nfp_nsp_hwinfo_set(nsp, hwinfo, sizeof(hwinfo)); - if (err) + if (err) { + /* Not a fatal error, no need to return error to stop driver from loading */ nfp_warn(pf->cpp, "HWinfo(sp_indiff=%d) set failed: %d\n", sp_indiff, err); + err = 0; + } nfp_nsp_close(nsp); return err; @@ -331,7 +334,23 @@ static int nfp_net_pf_cfg_nsp(struct nfp_pf *pf, bool sp_indiff) static int nfp_net_pf_init_nsp(struct nfp_pf *pf) { - return nfp_net_pf_cfg_nsp(pf, pf->sp_indiff); + int err; + + err = nfp_net_pf_cfg_nsp(pf, pf->sp_indiff); + if (!err) { + struct nfp_port *port; + + /* The eth ports need be refreshed after nsp is configured, + * since the eth table state may change, e.g. aneg_supp field. + * Only `CHANGED` bit is set here in case nsp needs some time + * to process the configuration. + */ + list_for_each_entry(port, &pf->ports, port_list) + if (__nfp_port_get_eth_port(port)) + set_bit(NFP_PORT_CHANGED, &port->flags); + } + + return err; } static void nfp_net_pf_clean_nsp(struct nfp_pf *pf) diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h index 52465670a01e..e045b6fb5fde 100644 --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h @@ -174,6 +174,7 @@ struct nfp_eth_table { bool enabled; bool tx_enabled; bool rx_enabled; + bool supp_aneg; bool override_changed; @@ -218,6 +219,7 @@ void nfp_eth_config_cleanup_end(struct nfp_nsp *nsp); int __nfp_eth_set_aneg(struct nfp_nsp *nsp, enum nfp_eth_aneg mode); int __nfp_eth_set_speed(struct nfp_nsp *nsp, unsigned int speed); int __nfp_eth_set_split(struct nfp_nsp *nsp, unsigned int lanes); +int __nfp_eth_set_fec(struct nfp_nsp *nsp, enum nfp_eth_fec mode); /** * struct nfp_nsp_identify - NSP static information diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c index 18ba7629cdc2..8084d52ade46 100644 --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c @@ -27,6 +27,7 @@ #define NSP_ETH_PORT_PHYLABEL GENMASK_ULL(59, 54) #define NSP_ETH_PORT_FEC_SUPP_BASER BIT_ULL(60) #define NSP_ETH_PORT_FEC_SUPP_RS BIT_ULL(61) +#define NSP_ETH_PORT_SUPP_ANEG BIT_ULL(63) #define NSP_ETH_PORT_LANES_MASK cpu_to_le64(NSP_ETH_PORT_LANES) @@ -178,6 +179,7 @@ nfp_eth_port_translate(struct nfp_nsp *nsp, const union eth_table_entry *src, return; dst->act_fec = FIELD_GET(NSP_ETH_STATE_ACT_FEC, state); + dst->supp_aneg = FIELD_GET(NSP_ETH_PORT_SUPP_ANEG, port); } static void @@ -564,7 +566,7 @@ int __nfp_eth_set_aneg(struct nfp_nsp *nsp, enum nfp_eth_aneg mode) * * Return: 0 or -ERRNO. */ -static int __nfp_eth_set_fec(struct nfp_nsp *nsp, enum nfp_eth_fec mode) +int __nfp_eth_set_fec(struct nfp_nsp *nsp, enum nfp_eth_fec mode) { return NFP_ETH_SET_BIT_CONFIG(nsp, NSP_ETH_RAW_STATE, NSP_ETH_STATE_FEC, mode, -- 2.30.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 2/3] nfp: add support for link auto negotiation 2022-09-21 12:12 ` [PATCH net-next 2/3] nfp: add support for link auto negotiation Simon Horman @ 2022-09-23 1:00 ` Jakub Kicinski 2022-09-23 4:37 ` Yinjun Zhang 0 siblings, 1 reply; 17+ messages in thread From: Jakub Kicinski @ 2022-09-23 1:00 UTC (permalink / raw) To: Simon Horman; +Cc: David Miller, Paolo Abeni, netdev, oss-drivers, Fei Qin On Wed, 21 Sep 2022 14:12:34 +0200 Simon Horman wrote: > From: Yinjun Zhang <yinjun.zhang@corigine.com> > > Report the auto negotiation capability if it's supported > in management firmware, and advertise it if it's enabled. > > Changing FEC to mode other than auto or changing port > speed is not allowed when autoneg is enabled. And FEC mode > is enforced into auto mode when enabling link autoneg. > > The ethtool <intf> command displays the auto-neg capability: > if (cmd->base.speed != SPEED_UNKNOWN) { > - u32 speed = cmd->base.speed / eth_port->lanes; > + if (req_aneg) { > + netdev_err(netdev, "Speed changing is not allowed when working on autoneg mode.\n"); > + err = -EINVAL; > + goto err_bad_set; > + } else { > + u32 speed = cmd->base.speed / eth_port->lanes; > > - err = __nfp_eth_set_speed(nsp, speed); > + err = __nfp_eth_set_speed(nsp, speed); > + if (err) > + goto err_bad_set; > + } Please refactor this to avoid the extra indentation > + } > + > + if (req_aneg && nfp_eth_can_support_fec(eth_port) && eth_port->fec != NFP_FEC_AUTO_BIT) { > + err = __nfp_eth_set_fec(nsp, NFP_FEC_AUTO_BIT); > if (err) > goto err_bad_set; > + if (eth_port->supp_aneg && eth_port->aneg == NFP_ANEG_AUTO && fec != NFP_FEC_AUTO_BIT) { > + netdev_err(netdev, "Only auto mode is allowed when link autoneg is enabled.\n"); > + return -EINVAL; > + } Autoneg and AUTO fec are two completely different things. There was a long thread on AUTO recently.. :( > snprintf(hwinfo, sizeof(hwinfo), "sp_indiff=%d", sp_indiff); > err = nfp_nsp_hwinfo_set(nsp, hwinfo, sizeof(hwinfo)); > - if (err) > + if (err) { > + /* Not a fatal error, no need to return error to stop driver from loading */ > nfp_warn(pf->cpp, "HWinfo(sp_indiff=%d) set failed: %d\n", sp_indiff, err); > + err = 0; This should be a separate commit, it seems > > nfp_nsp_close(nsp); > return err; > @@ -331,7 +334,23 @@ static int nfp_net_pf_cfg_nsp(struct nfp_pf *pf, bool sp_indiff) > > static int nfp_net_pf_init_nsp(struct nfp_pf *pf) > { > - return nfp_net_pf_cfg_nsp(pf, pf->sp_indiff); > + int err; > + > + err = nfp_net_pf_cfg_nsp(pf, pf->sp_indiff); > + if (!err) { > + struct nfp_port *port; > + > + /* The eth ports need be refreshed after nsp is configured, > + * since the eth table state may change, e.g. aneg_supp field. No idea why, tho > + * Only `CHANGED` bit is set here in case nsp needs some time > + * to process the configuration. I can't parse what this is saying but doesn't look good > + */ > + list_for_each_entry(port, &pf->ports, port_list) > + if (__nfp_port_get_eth_port(port)) > + set_bit(NFP_PORT_CHANGED, &port->flags); > + } > + > + return err; > } ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH net-next 2/3] nfp: add support for link auto negotiation 2022-09-23 1:00 ` Jakub Kicinski @ 2022-09-23 4:37 ` Yinjun Zhang 2022-09-23 13:21 ` Jakub Kicinski 0 siblings, 1 reply; 17+ messages in thread From: Yinjun Zhang @ 2022-09-23 4:37 UTC (permalink / raw) To: Jakub Kicinski, Simon Horman Cc: David Miller, Paolo Abeni, netdev@vger.kernel.org, oss-drivers, Fei Qin On Thu, 22 Sep 2022 18:00:40 -0700 Jakub Kicinski wrote: > On Wed, 21 Sep 2022 14:12:34 +0200 Simon Horman wrote: > > From: Yinjun Zhang <yinjun.zhang@corigine.com> > > > > Report the auto negotiation capability if it's supported > > in management firmware, and advertise it if it's enabled. > > > > Changing FEC to mode other than auto or changing port > > speed is not allowed when autoneg is enabled. And FEC mode > > is enforced into auto mode when enabling link autoneg. > > > > The ethtool <intf> command displays the auto-neg capability: > > > + goto err_bad_set; > > + } > > Please refactor this to avoid the extra indentation Will do. > > > + if (eth_port->supp_aneg && eth_port->aneg == NFP_ANEG_AUTO && > fec != NFP_FEC_AUTO_BIT) { > > + netdev_err(netdev, "Only auto mode is allowed when link > autoneg is enabled.\n"); > > + return -EINVAL; > > + } > > Autoneg and AUTO fec are two completely different things. > There was a long thread on AUTO recently.. :( Yes, you're right, will remove this limitation. > > > + /* Not a fatal error, no need to return error to stop driver > from loading */ > > nfp_warn(pf->cpp, "HWinfo(sp_indiff=%d) set failed: %d\n", > sp_indiff, err); > > + err = 0; > > This should be a separate commit, it seems Will separate it. > > > > > nfp_nsp_close(nsp); > > return err; > > @@ -331,7 +334,23 @@ static int nfp_net_pf_cfg_nsp(struct nfp_pf *pf, > bool sp_indiff) > > > > static int nfp_net_pf_init_nsp(struct nfp_pf *pf) > > { > > - return nfp_net_pf_cfg_nsp(pf, pf->sp_indiff); > > + int err; > > + > > + err = nfp_net_pf_cfg_nsp(pf, pf->sp_indiff); > > + if (!err) { > > + struct nfp_port *port; > > + > > + /* The eth ports need be refreshed after nsp is configured, > > + * since the eth table state may change, e.g. aneg_supp field. > > No idea why, tho > > > + * Only `CHANGED` bit is set here in case nsp needs some > time > > + * to process the configuration. > > I can't parse what this is saying but doesn't look good I think this comment is clear enough. In previous ` nfp_net_pf_cfg_nsp`, hwinfo "sp_indiff" is configured into Management firmware(NSP), and it decides if autoneg is supported or not and updates eth table accordingly. And only `CHANGED` flag is set here so that with some delay driver can get the updated eth table instead of stale info. > > > + */ > > + list_for_each_entry(port, &pf->ports, port_list) > > + if (__nfp_port_get_eth_port(port)) > > + set_bit(NFP_PORT_CHANGED, &port->flags); > > + } > > + > > + return err; > > } ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 2/3] nfp: add support for link auto negotiation 2022-09-23 4:37 ` Yinjun Zhang @ 2022-09-23 13:21 ` Jakub Kicinski 2022-09-23 15:41 ` Yinjun Zhang 0 siblings, 1 reply; 17+ messages in thread From: Jakub Kicinski @ 2022-09-23 13:21 UTC (permalink / raw) To: Yinjun Zhang Cc: Simon Horman, David Miller, Paolo Abeni, netdev@vger.kernel.org, oss-drivers, Fei Qin On Fri, 23 Sep 2022 04:37:58 +0000 Yinjun Zhang wrote: > > I can't parse what this is saying but doesn't look good > > I think this comment is clear enough. In previous ` > nfp_net_pf_cfg_nsp`, hwinfo "sp_indiff" is configured into Management > firmware(NSP), and it decides if autoneg is supported or not and > updates eth table accordingly. And only `CHANGED` flag is set here so > that with some delay driver can get the updated eth table instead of > stale info. Why is the sp_indif thing configured at the nfp_main layer, before the eth table is read? Doing this inside nfp_net_main seems like the wrong layering to me. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 2/3] nfp: add support for link auto negotiation 2022-09-23 13:21 ` Jakub Kicinski @ 2022-09-23 15:41 ` Yinjun Zhang 2022-09-24 0:24 ` Jakub Kicinski 0 siblings, 1 reply; 17+ messages in thread From: Yinjun Zhang @ 2022-09-23 15:41 UTC (permalink / raw) To: Jakub Kicinski Cc: Simon Horman, David Miller, Paolo Abeni, netdev@vger.kernel.org, oss-drivers, Fei Qin On Fri, Sep 23, 2022 at 06:21:14AM -0700, Jakub Kicinski wrote: > On Fri, 23 Sep 2022 04:37:58 +0000 Yinjun Zhang wrote: > > > I can't parse what this is saying but doesn't look good > > > > I think this comment is clear enough. In previous ` > > nfp_net_pf_cfg_nsp`, hwinfo "sp_indiff" is configured into Management > > firmware(NSP), and it decides if autoneg is supported or not and > > updates eth table accordingly. And only `CHANGED` flag is set here so > > that with some delay driver can get the updated eth table instead of > > stale info. > > Why is the sp_indif thing configured at the nfp_main layer, before > the eth table is read? Doing this inside nfp_net_main seems like > the wrong layering to me. Because the value of sp_indiff depends on the loaded application firmware, please ref to previous commit: 2b88354d37ca ("nfp: check if application firmware is indifferent to port speed") ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 2/3] nfp: add support for link auto negotiation 2022-09-23 15:41 ` Yinjun Zhang @ 2022-09-24 0:24 ` Jakub Kicinski 2022-09-24 2:45 ` Yinjun Zhang 0 siblings, 1 reply; 17+ messages in thread From: Jakub Kicinski @ 2022-09-24 0:24 UTC (permalink / raw) To: Yinjun Zhang Cc: Simon Horman, David Miller, Paolo Abeni, netdev@vger.kernel.org, oss-drivers, Fei Qin On Fri, 23 Sep 2022 23:41:57 +0800 Yinjun Zhang wrote: > > Why is the sp_indif thing configured at the nfp_main layer, before > > the eth table is read? Doing this inside nfp_net_main seems like > > the wrong layering to me. > > Because the value of sp_indiff depends on the loaded application > firmware, please ref to previous commit: > 2b88354d37ca ("nfp: check if application firmware is indifferent to port speed") AFAICT you check if it's flower, you can check that in the main code, the app id is just a symbol, right? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 2/3] nfp: add support for link auto negotiation 2022-09-24 0:24 ` Jakub Kicinski @ 2022-09-24 2:45 ` Yinjun Zhang 2022-09-26 16:25 ` Jakub Kicinski 0 siblings, 1 reply; 17+ messages in thread From: Yinjun Zhang @ 2022-09-24 2:45 UTC (permalink / raw) To: Jakub Kicinski Cc: Simon Horman, David Miller, Paolo Abeni, netdev@vger.kernel.org, oss-drivers, Fei Qin On Fri, Sep 23, 2022 at 05:24:10PM -0700, Jakub Kicinski wrote: > On Fri, 23 Sep 2022 23:41:57 +0800 Yinjun Zhang wrote: > > > Why is the sp_indif thing configured at the nfp_main layer, before > > > the eth table is read? Doing this inside nfp_net_main seems like > > > the wrong layering to me. > > > > Because the value of sp_indiff depends on the loaded application > > firmware, please ref to previous commit: > > 2b88354d37ca ("nfp: check if application firmware is indifferent to port speed") > > AFAICT you check if it's flower, you can check that in the main code, > the app id is just a symbol, right? Not only check if it's flower, but also check if it's sp_indiff when it's not flower by parsing the tlv caps. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 2/3] nfp: add support for link auto negotiation 2022-09-24 2:45 ` Yinjun Zhang @ 2022-09-26 16:25 ` Jakub Kicinski 2022-09-27 1:13 ` Yinjun Zhang 0 siblings, 1 reply; 17+ messages in thread From: Jakub Kicinski @ 2022-09-26 16:25 UTC (permalink / raw) To: Yinjun Zhang Cc: Simon Horman, David Miller, Paolo Abeni, netdev@vger.kernel.org, oss-drivers, Fei Qin On Sat, 24 Sep 2022 10:45:30 +0800 Yinjun Zhang wrote: > On Fri, Sep 23, 2022 at 05:24:10PM -0700, Jakub Kicinski wrote: > > > Because the value of sp_indiff depends on the loaded application > > > firmware, please ref to previous commit: > > > 2b88354d37ca ("nfp: check if application firmware is indifferent to port speed") > > > > AFAICT you check if it's flower, you can check that in the main code, > > the app id is just a symbol, right? > > Not only check if it's flower, but also check if it's sp_indiff when > it's not flower by parsing the tlv caps. Seems bogus. The speed independence is a property of the whole FW image, you record it in the pf structure. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 2/3] nfp: add support for link auto negotiation 2022-09-26 16:25 ` Jakub Kicinski @ 2022-09-27 1:13 ` Yinjun Zhang 2022-09-27 1:38 ` Jakub Kicinski 0 siblings, 1 reply; 17+ messages in thread From: Yinjun Zhang @ 2022-09-27 1:13 UTC (permalink / raw) To: Jakub Kicinski Cc: Simon Horman, David Miller, Paolo Abeni, netdev@vger.kernel.org, oss-drivers, Fei Qin On Mon, Sep 26, 2022 at 09:25:47AM -0700, Jakub Kicinski wrote: > On Sat, 24 Sep 2022 10:45:30 +0800 Yinjun Zhang wrote: > > On Fri, Sep 23, 2022 at 05:24:10PM -0700, Jakub Kicinski wrote: > > > > Because the value of sp_indiff depends on the loaded application > > > > firmware, please ref to previous commit: > > > > 2b88354d37ca ("nfp: check if application firmware is indifferent to port speed") > > > > > > AFAICT you check if it's flower, you can check that in the main code, > > > the app id is just a symbol, right? > > > > Not only check if it's flower, but also check if it's sp_indiff when > > it's not flower by parsing the tlv caps. > > Seems bogus. The speed independence is a property of the whole FW image, > you record it in the pf structure. It's indeed a per-fw property, but we don't have existing way to expose per-fw capabilities to driver currently, so use per-vnic tlv caps here. Maybe define a new fw symbol is a choice, but my concern is it's not visible to netvf driver. Any suggestion is welcomed. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 2/3] nfp: add support for link auto negotiation 2022-09-27 1:13 ` Yinjun Zhang @ 2022-09-27 1:38 ` Jakub Kicinski 2022-09-27 1:52 ` Yinjun Zhang 0 siblings, 1 reply; 17+ messages in thread From: Jakub Kicinski @ 2022-09-27 1:38 UTC (permalink / raw) To: Yinjun Zhang Cc: Simon Horman, David Miller, Paolo Abeni, netdev@vger.kernel.org, oss-drivers, Fei Qin On Tue, 27 Sep 2022 09:13:53 +0800 Yinjun Zhang wrote: > On Mon, Sep 26, 2022 at 09:25:47AM -0700, Jakub Kicinski wrote: > > On Sat, 24 Sep 2022 10:45:30 +0800 Yinjun Zhang wrote: > > > Not only check if it's flower, but also check if it's sp_indiff when > > > it's not flower by parsing the tlv caps. > > > > Seems bogus. The speed independence is a property of the whole FW image, > > you record it in the pf structure. > > It's indeed a per-fw property, but we don't have existing way to expose > per-fw capabilities to driver currently, so use per-vnic tlv caps here. > Maybe define a new fw symbol is a choice, but my concern is it's not > visible to netvf driver. > Any suggestion is welcomed. Why not put an rtsym with the value in the FW? That'd be my first go-to way of communicating information about the FW as a whole. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 2/3] nfp: add support for link auto negotiation 2022-09-27 1:38 ` Jakub Kicinski @ 2022-09-27 1:52 ` Yinjun Zhang 0 siblings, 0 replies; 17+ messages in thread From: Yinjun Zhang @ 2022-09-27 1:52 UTC (permalink / raw) To: Jakub Kicinski Cc: Simon Horman, David Miller, Paolo Abeni, netdev@vger.kernel.org, oss-drivers, Fei Qin On Mon, Sep 26, 2022 at 06:38:40PM -0700, Jakub Kicinski wrote: > On Tue, 27 Sep 2022 09:13:53 +0800 Yinjun Zhang wrote: > > On Mon, Sep 26, 2022 at 09:25:47AM -0700, Jakub Kicinski wrote: > > > On Sat, 24 Sep 2022 10:45:30 +0800 Yinjun Zhang wrote: > > > > Not only check if it's flower, but also check if it's sp_indiff when > > > > it's not flower by parsing the tlv caps. > > > > > > Seems bogus. The speed independence is a property of the whole FW image, > > > you record it in the pf structure. > > > > It's indeed a per-fw property, but we don't have existing way to expose > > per-fw capabilities to driver currently, so use per-vnic tlv caps here. > > Maybe define a new fw symbol is a choice, but my concern is it's not > > visible to netvf driver. > > Any suggestion is welcomed. > > Why not put an rtsym with the value in the FW? That'd be my first > go-to way of communicating information about the FW as a whole. Like I said, the VF driver cannot read the rtsym, so it's not a perfect way as exposing a per-FW property. But for this case, it's OK since VF doesn't need this info. I'll try this way. Thanks. ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next 3/3] nfp: add support restart of link auto-negotiation 2022-09-21 12:12 [PATCH net-next 0/3] nfp: support FEC mode reporting and auto-neg Simon Horman 2022-09-21 12:12 ` [PATCH net-next 1/3] nfp: add support for reporting active FEC mode Simon Horman 2022-09-21 12:12 ` [PATCH net-next 2/3] nfp: add support for link auto negotiation Simon Horman @ 2022-09-21 12:12 ` Simon Horman 2022-09-23 0:54 ` Jakub Kicinski 2 siblings, 1 reply; 17+ messages in thread From: Simon Horman @ 2022-09-21 12:12 UTC (permalink / raw) To: David Miller, Jakub Kicinski, Paolo Abeni; +Cc: netdev, oss-drivers, Fei Qin From: Fei Qin <fei.qin@corigine.com> Add support restart of link auto-negotiation. This may be initiated using: # ethtool -r <intf> Signed-off-by: Fei Qin <fei.qin@corigine.com> Reviewed-by: Louis Peens <louis.peens@corigine.com> Signed-off-by: Simon Horman <simon.horman@corigine.com> --- .../ethernet/netronome/nfp/nfp_net_ethtool.c | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c index 00aacc48a7a2..ceda4d0b98bf 100644 --- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c @@ -228,6 +228,37 @@ nfp_net_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo) nfp_get_drvinfo(nn->app, nn->pdev, vnic_version, drvinfo); } +static int +nfp_net_nway_reset(struct net_device *netdev) +{ + struct nfp_eth_table_port *eth_port; + struct nfp_port *port; + int err; + + port = nfp_port_from_netdev(netdev); + eth_port = nfp_port_get_eth_port(port); + if (!eth_port) + return -EOPNOTSUPP; + + if (!netif_running(netdev)) + return 0; + + err = nfp_port_configure(netdev, false); + if (err) { + netdev_info(netdev, "Link down failed: %d\n", err); + return err; + } + + err = nfp_port_configure(netdev, true); + if (err) { + netdev_info(netdev, "Link up failed: %d\n", err); + return err; + } + + netdev_info(netdev, "Link reset succeeded\n"); + return 0; +} + static void nfp_app_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo) { @@ -1854,6 +1885,7 @@ static const struct ethtool_ops nfp_net_ethtool_ops = { ETHTOOL_COALESCE_MAX_FRAMES | ETHTOOL_COALESCE_USE_ADAPTIVE, .get_drvinfo = nfp_net_get_drvinfo, + .nway_reset = nfp_net_nway_reset, .get_link = ethtool_op_get_link, .get_ringparam = nfp_net_get_ringparam, .set_ringparam = nfp_net_set_ringparam, @@ -1891,6 +1923,7 @@ static const struct ethtool_ops nfp_net_ethtool_ops = { const struct ethtool_ops nfp_port_ethtool_ops = { .get_drvinfo = nfp_app_get_drvinfo, + .nway_reset = nfp_net_nway_reset, .get_link = ethtool_op_get_link, .get_strings = nfp_port_get_strings, .get_ethtool_stats = nfp_port_get_stats, -- 2.30.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 3/3] nfp: add support restart of link auto-negotiation 2022-09-21 12:12 ` [PATCH net-next 3/3] nfp: add support restart of link auto-negotiation Simon Horman @ 2022-09-23 0:54 ` Jakub Kicinski 2022-09-23 4:40 ` Yinjun Zhang 0 siblings, 1 reply; 17+ messages in thread From: Jakub Kicinski @ 2022-09-23 0:54 UTC (permalink / raw) To: Simon Horman; +Cc: David Miller, Paolo Abeni, netdev, oss-drivers, Fei Qin On Wed, 21 Sep 2022 14:12:35 +0200 Simon Horman wrote: > + err = nfp_port_configure(netdev, false); > + if (err) { > + netdev_info(netdev, "Link down failed: %d\n", err); > + return err; > + } > + > + err = nfp_port_configure(netdev, true); > + if (err) { > + netdev_info(netdev, "Link up failed: %d\n", err); > + return err; > + } > + > + netdev_info(netdev, "Link reset succeeded\n"); > + return 0; This will not do anything if the port is forced up by multi host or NC-SI. ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH net-next 3/3] nfp: add support restart of link auto-negotiation 2022-09-23 0:54 ` Jakub Kicinski @ 2022-09-23 4:40 ` Yinjun Zhang 2022-09-23 4:52 ` Yinjun Zhang 0 siblings, 1 reply; 17+ messages in thread From: Yinjun Zhang @ 2022-09-23 4:40 UTC (permalink / raw) To: Jakub Kicinski, Simon Horman Cc: David Miller, Paolo Abeni, netdev@vger.kernel.org, oss-drivers, Fei Qin On Thu, 22 Sep 2022 17:54:53 -0700 Jakub Kicinski wrote: > On Wed, 21 Sep 2022 14:12:35 +0200 Simon Horman wrote: > > + > > + netdev_info(netdev, "Link reset succeeded\n"); > > + return 0; > > This will not do anything if the port is forced up by multi host > or NC-SI. OK, we're going to do the reset thing as long as it's physically configured up. ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH net-next 3/3] nfp: add support restart of link auto-negotiation 2022-09-23 4:40 ` Yinjun Zhang @ 2022-09-23 4:52 ` Yinjun Zhang 0 siblings, 0 replies; 17+ messages in thread From: Yinjun Zhang @ 2022-09-23 4:52 UTC (permalink / raw) To: Jakub Kicinski, Simon Horman Cc: David Miller, Paolo Abeni, netdev@vger.kernel.org, oss-drivers, Fei Qin > On Thu, 22 Sep 2022 17:54:53 -0700 Jakub Kicinski wrote: > > On Wed, 21 Sep 2022 14:12:35 +0200 Simon Horman wrote: > > > + > > > + netdev_info(netdev, "Link reset succeeded\n"); > > > + return 0; > > > > This will not do anything if the port is forced up by multi host > > or NC-SI. > > OK, we're going to do the reset thing as long as it's physically > configured up. Sorry, I may misunderstand, will use ` nfp_eth_set_configured` directly. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-09-27 1:52 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-09-21 12:12 [PATCH net-next 0/3] nfp: support FEC mode reporting and auto-neg Simon Horman 2022-09-21 12:12 ` [PATCH net-next 1/3] nfp: add support for reporting active FEC mode Simon Horman 2022-09-21 12:12 ` [PATCH net-next 2/3] nfp: add support for link auto negotiation Simon Horman 2022-09-23 1:00 ` Jakub Kicinski 2022-09-23 4:37 ` Yinjun Zhang 2022-09-23 13:21 ` Jakub Kicinski 2022-09-23 15:41 ` Yinjun Zhang 2022-09-24 0:24 ` Jakub Kicinski 2022-09-24 2:45 ` Yinjun Zhang 2022-09-26 16:25 ` Jakub Kicinski 2022-09-27 1:13 ` Yinjun Zhang 2022-09-27 1:38 ` Jakub Kicinski 2022-09-27 1:52 ` Yinjun Zhang 2022-09-21 12:12 ` [PATCH net-next 3/3] nfp: add support restart of link auto-negotiation Simon Horman 2022-09-23 0:54 ` Jakub Kicinski 2022-09-23 4:40 ` Yinjun Zhang 2022-09-23 4:52 ` Yinjun Zhang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).