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

From: Sabrina Dubroca
Date: Mon Oct 02 2023 - 17:08:47 EST


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