Re: [PATCH net-next v6 08/10] net: phy: nxp-c45-tja11xx: add MACsec support

[Sabrina Dubroca <sd@queasysnail.net>]

2023-09-28, 11:44:28 +0300, Radu Pirea (NXP OSS) wrote:
> +static int nxp_c45_mdo_upd_secy(struct macsec_context *ctx)
> +{
> +	u8 encoding_sa = ctx->secy->tx_sc.encoding_sa;
> +	struct phy_device *phydev = ctx->phydev;
> +	struct nxp_c45_phy *priv = phydev->priv;
> +	struct nxp_c45_secy *phy_secy;
> +	struct nxp_c45_sa next_sa;
> +	bool can_rx_sc0_impl;
> +
> +	phydev_dbg(phydev, "update SecY SCI %016llx\n",
> +		   sci_to_cpu(ctx->secy->sci));
> +
> +	phy_secy = nxp_c45_find_secy(&priv->macsec->secy_list, ctx->secy->sci);
> +	if (IS_ERR(phy_secy))
> +		return PTR_ERR(phy_secy);
> +
> +	if (!nxp_c45_mac_addr_free(ctx))
> +		return -EBUSY;

mdo_upd_secy gets called from macsec_set_mac_address, but the error is ignored:

	static int macsec_set_mac_address(struct net_device *dev, void *p)
	{
	[...]
		/* If h/w offloading is available, propagate to the device */
		if (macsec_is_offloaded(macsec)) {
			const struct macsec_ops *ops;
			struct macsec_context ctx;
	
			ops = macsec_get_ops(macsec, &ctx);
			if (ops) {
				ctx.secy = &macsec->secy;
				macsec_offload(ops->mdo_upd_secy, &ctx);
			}
		}
	
		return 0;
	}


Should macsec_set_mac_address try to roll back the change when
mdo_upd_secy fails? Otherwise I guess your device doesn't work.


> +static int nxp_c45_mdo_add_txsa(struct macsec_context *ctx)
> +{
...
> +	nxp_c45_select_secy(phydev, phy_secy->secy_id);
> +	nxp_c45_sa_set_pn(phydev, sa, tx_sa->next_pn, 0);
> +	nxp_c45_sa_set_key(ctx, sa->regs, tx_sa->key.salt.bytes, tx_sa->ssci);
> +	if (ctx->secy->tx_sc.encoding_sa  == sa->an)

nit: double ' ' before '==' (also in nxp_c45_mdo_del_txsa)

-- 
Sabrina