Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Date:   Mon, 2 Oct 2023 23:08:14 +0200
From:   Sabrina Dubroca <sd@queasysnail.net>
To:     "Radu Pirea (NXP OSS)" <radu-nicolae.pirea@oss.nxp.com>
Cc:     sgoutham@marvell.com, gakula@marvell.com, sbhatta@marvell.com,
        hkelam@marvell.com, davem@davemloft.net, edumazet@google.com,
        kuba@kernel.org, pabeni@redhat.com, borisp@nvidia.com,
        saeedm@nvidia.com, leon@kernel.org, andrew@lunn.ch,
        hkallweit1@gmail.com, linux@armlinux.org.uk,
        richardcochran@gmail.com, sebastian.tobuschat@oss.nxp.com,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        linux-rdma@vger.kernel.org
Subject: Re: [PATCH net-next v6 08/10] net: phy: nxp-c45-tja11xx: add MACsec
 support
Message-ID: <ZRsxPvGXJAbgkzYL@hog>
References: <20230928084430.1882670-1-radu-nicolae.pirea@oss.nxp.com>
 <20230928084430.1882670-9-radu-nicolae.pirea@oss.nxp.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20230928084430.1882670-9-radu-nicolae.pirea@oss.nxp.com>
Precedence: bulk

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