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: <DM6PR13MB3705B174455A7E5225CAF996FC519@DM6PR13MB3705.namprd13.prod.outlook.com> Date: Fri, 23 Sep 2022 04:37:58 +0000 From: Yinjun Zhang <yinjun.zhang@...igine.com> To: Jakub Kicinski <kuba@...nel.org>, Simon Horman <simon.horman@...igine.com> CC: David Miller <davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, oss-drivers <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 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@...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: > > > + 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; > > }
Powered by blists - more mailing lists