lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20220922180040.50dd1af0@kernel.org> Date: Thu, 22 Sep 2022 18:00:40 -0700 From: Jakub Kicinski <kuba@...nel.org> To: Simon Horman <simon.horman@...igine.com> Cc: David Miller <davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, oss-drivers@...igine.com, Fei Qin <fei.qin@...igine.com> Subject: Re: [PATCH net-next 2/3] nfp: add support for link auto negotiation On Wed, 21 Sep 2022 14:12:34 +0200 Simon Horman wrote: > From: Yinjun Zhang <yinjun.zhang@...igine.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; > }
Powered by blists - more mailing lists